Reland: Move geolocation unittests from //device to //services

This is one step of "moving //device/geolocation into //services". TBR=benchan@chromium.org BUG=800659 Change-Id: I566db835e7df596374cd341048d9734bf82398f6 Reviewed-on: https://chromium-review.googlesource.com/1107637 Reviewed-by: Ben Chan <benchan@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: Ke He <ke.he@intel.com> Cr-Commit-Position: refs/heads/master@{#571374}
Denger-Network · Jun 29, 2018 · 1082a6a · 1082a6a
1 parent 043152d
commit 1082a6a
Show file tree

Hide file tree

Showing 15 changed files with 50 additions and 75 deletions.
diff --git a/device/BUILD.gn b/device/BUILD.gn
@@ -114,7 +114,6 @@ test("device_unittests") {
     "//device/gamepad/public/cpp:shared_with_blink",
     "//device/gamepad/public/mojom",
     "//device/gamepad/public/mojom:gamepad_mojom_traits_test",
-    "//device/geolocation:unittests",
     "//mojo/edk",
     "//mojo/public/cpp/bindings",
     "//net",

diff --git a/device/geolocation/BUILD.gn b/device/geolocation/BUILD.gn
@@ -149,52 +149,3 @@ source_set("test_support") {
     "//testing/gtest",
   ]
 }
-
-source_set("unittests") {
-  testonly = true
-
-  sources = [
-    "geolocation_provider_impl_unittest.cc",
-    "location_arbitrator_unittest.cc",
-    "network_location_provider_unittest.cc",
-    "wifi_data_provider_chromeos_unittest.cc",
-    "wifi_data_provider_common_unittest.cc",
-    "wifi_data_provider_linux_unittest.cc",
-    "wifi_data_provider_win_unittest.cc",
-    "wifi_polling_policy_unittest.cc",
-  ]
-  public_deps = [
-    ":geolocation",
-  ]
-  deps = [
-    ":test_support",
-    "//base",
-    "//base/third_party/dynamic_annotations",
-    "//device/geolocation/public/cpp",
-    "//net:test_support",
-    "//services/device/public/mojom",
-    "//testing/gmock",
-    "//testing/gtest",
-  ]
-
-  if (is_linux) {
-    if (use_dbus) {
-      deps += [ "//dbus:test_support" ]
-    } else {
-      sources -= [ "wifi_data_provider_linux_unittest.cc" ]
-    }
-  }
-
-  if (is_chromeos) {
-    sources -= [ "wifi_data_provider_linux_unittest.cc" ]
-    deps += [ "//chromeos" ]
-  }
-
-  if (is_android) {
-    sources -= [
-      "network_location_provider_unittest.cc",
-      "wifi_data_provider_common_unittest.cc",
-    ]
-    deps += [ "//device/geolocation/public/java:geolocation_java_test_support" ]
-  }
-}
diff --git a/device/geolocation/geolocation_provider_impl.h b/device/geolocation/geolocation_provider_impl.h
@@ -79,6 +79,10 @@ class GeolocationProviderImpl : public GeolocationProvider,
     return user_did_opt_into_location_services_;
   }
 
+  void clear_user_did_opt_into_location_services_for_testing() {
+    user_did_opt_into_location_services_ = false;
+  }
+
   // Safe to call while there are no GeolocationProviderImpl clients
   // registered.
   void SetArbitratorForTesting(std::unique_ptr<LocationProvider> arbitrator);

diff --git a/services/device/BUILD.gn b/services/device/BUILD.gn
@@ -84,9 +84,16 @@ source_set("tests") {
     "generic_sensor/platform_sensor_provider_unittest_android.cc",
     "generic_sensor/relative_orientation_euler_angles_fusion_algorithm_using_accelerometer_and_gyroscope_unittest.cc",
     "generic_sensor/relative_orientation_euler_angles_fusion_algorithm_using_accelerometer_unittest.cc",
+    "geolocation/geolocation_provider_impl_unittest.cc",
     "geolocation/geolocation_service_unittest.cc",
+    "geolocation/location_arbitrator_unittest.cc",
+    "geolocation/network_location_provider_unittest.cc",
     "geolocation/public_ip_address_geolocator_unittest.cc",
     "geolocation/public_ip_address_location_notifier_unittest.cc",
+    "geolocation/wifi_data_provider_chromeos_unittest.cc",
+    "geolocation/wifi_data_provider_common_unittest.cc",
+    "geolocation/wifi_data_provider_win_unittest.cc",
+    "geolocation/wifi_polling_policy_unittest.cc",
     "power_monitor/power_monitor_message_broadcaster_unittest.cc",
     "public/cpp/power_monitor/power_monitor_broadcast_source_unittest.cc",
     "vibration/vibration_manager_impl_unittest.cc",
@@ -97,9 +104,9 @@ source_set("tests") {
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//base/third_party/dynamic_annotations",
     "//device/base/synchronization",
-    "//device/geolocation",
-    "//device/geolocation/public/cpp",
+    "//device/geolocation:test_support",
     "//mojo/edk",
     "//mojo/public/cpp/bindings",
     "//net",
@@ -121,7 +128,10 @@ source_set("tests") {
   }
 
   if (is_linux && !is_chromeos && use_dbus) {
-    sources += [ "battery/battery_status_manager_linux_unittest.cc" ]
+    sources += [
+      "battery/battery_status_manager_linux_unittest.cc",
+      "geolocation/wifi_data_provider_linux_unittest.cc",
+    ]
     deps += [ "//dbus:test_support" ]
   }
 
@@ -136,9 +146,14 @@ source_set("tests") {
   }
 
   if (is_android) {
-    sources -= [ "battery/battery_status_service_unittest.cc" ]
+    sources -= [
+      "battery/battery_status_service_unittest.cc",
+      "geolocation/network_location_provider_unittest.cc",
+      "geolocation/wifi_data_provider_common_unittest.cc",
+    ]
     deps += [
       ":device_service_jni_headers",
+      "//device/geolocation/public/java:geolocation_java_test_support",
       "//services/device/vibration/android:vibration_jni_headers",
     ]
   } else {

diff --git a/services/device/geolocation/DEPS b/services/device/geolocation/DEPS
@@ -1,4 +1,6 @@
 include_rules = [
   "+chromeos",
+  "+dbus",
   "+net",
+  "+third_party/cros_system_api/dbus",
 ]
diff --git a/...ion/geolocation_provider_impl_unittest.cc → ...ion/geolocation_provider_impl_unittest.cc b/...ion/geolocation_provider_impl_unittest.cc → ...ion/geolocation_provider_impl_unittest.cc
@@ -177,6 +177,7 @@ TEST_F(GeolocationProviderTest, OnPermissionGrantedWithoutObservers) {
   EXPECT_FALSE(provider()->user_did_opt_into_location_services_for_testing());
   provider()->UserDidOptIntoLocationServices();
   EXPECT_TRUE(provider()->user_did_opt_into_location_services_for_testing());
+  provider()->clear_user_did_opt_into_location_services_for_testing();
 }
 
 TEST_F(GeolocationProviderTest, StartStop) {

diff --git a/services/device/geolocation/geolocation_service_unittest.cc b/services/device/geolocation/geolocation_service_unittest.cc
@@ -96,6 +96,8 @@ class GeolocationServiceUnitTest : public DeviceServiceTestBase {
     // make sure the base::CallbackList<> member in GeolocationProviderImpl is
     // empty.
     geolocation_.reset();
+    GeolocationProviderImpl::GetInstance()
+        ->clear_user_did_opt_into_location_services_for_testing();
     base::RunLoop().RunUntilIdle();
   }
 

diff --git a/...olocation/location_arbitrator_unittest.cc → ...olocation/location_arbitrator_unittest.cc b/...olocation/location_arbitrator_unittest.cc → ...olocation/location_arbitrator_unittest.cc
@@ -77,6 +77,17 @@ void SetReferencePosition(FakeLocationProvider* provider) {
   SetPositionFix(provider, 51.0, -0.1, 400);
 }
 
+// Simple request context producer that immediately produces a
+// TestURLRequestContextGetter.
+void TestRequestContextProducer(
+    const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner,
+    base::OnceCallback<void(scoped_refptr<net::URLRequestContextGetter>)>
+        response_callback) {
+  std::move(response_callback)
+      .Run(base::MakeRefCounted<net::TestURLRequestContextGetter>(
+          network_task_runner));
+}
+
 }  // namespace
 
 // Simple request context producer that immediately produces a nullptr
@@ -89,17 +100,6 @@ void NullRequestContextProducer(
       .Run(scoped_refptr<net::URLRequestContextGetter>(nullptr));
 }
 
-// Simple request context producer that immediately produces a
-// TestURLRequestContextGetter.
-void TestRequestContextProducer(
-    const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner,
-    base::OnceCallback<void(scoped_refptr<net::URLRequestContextGetter>)>
-        response_callback) {
-  std::move(response_callback)
-      .Run(base::MakeRefCounted<net::TestURLRequestContextGetter>(
-          network_task_runner));
-}
-
 class TestingLocationArbitrator : public LocationArbitrator {
  public:
   TestingLocationArbitrator(

diff --git a/...ion/network_location_provider_unittest.cc → ...ion/network_location_provider_unittest.cc b/...ion/network_location_provider_unittest.cc → ...ion/network_location_provider_unittest.cc
@@ -229,13 +229,13 @@ class GeolocationNetworkProviderTest : public testing::Test {
     const base::Value* expected_value;
     const base::Value* actual_value;
     if (!expected.Get(field, &expected_value))
-      return testing::AssertionFailure() << "Expected dictionary "
-                                         << PrettyJson(expected)
-                                         << " is missing field " << field;
+      return testing::AssertionFailure()
+             << "Expected dictionary " << PrettyJson(expected)
+             << " is missing field " << field;
     if (!expected.Get(field, &actual_value))
-      return testing::AssertionFailure() << "Actual dictionary "
-                                         << PrettyJson(actual)
-                                         << " is missing field " << field;
+      return testing::AssertionFailure()
+             << "Actual dictionary " << PrettyJson(actual)
+             << " is missing field " << field;
     if (!expected_value->Equals(actual_value))
       return testing::AssertionFailure()
              << "Field " << field

diff --git a/services/device/geolocation/public_ip_address_location_notifier_unittest.cc b/services/device/geolocation/public_ip_address_location_notifier_unittest.cc
@@ -17,7 +17,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace device {
-
+namespace {
 // Simple request context producer that immediately produces a
 // TestURLRequestContextGetter.
 void TestRequestContextProducer(
@@ -29,6 +29,8 @@ void TestRequestContextProducer(
           network_task_runner));
 }
 
+}  // namespace
+
 class PublicIpAddressLocationNotifierTest : public testing::Test {
  protected:
   // Helps test a single call to

diff --git a/...n/wifi_data_provider_chromeos_unittest.cc → ...n/wifi_data_provider_chromeos_unittest.cc b/...n/wifi_data_provider_chromeos_unittest.cc → ...n/wifi_data_provider_chromeos_unittest.cc
diff --git a/...ion/wifi_data_provider_common_unittest.cc → ...ion/wifi_data_provider_common_unittest.cc b/...ion/wifi_data_provider_common_unittest.cc → ...ion/wifi_data_provider_common_unittest.cc
diff --git a/...tion/wifi_data_provider_linux_unittest.cc → ...tion/wifi_data_provider_linux_unittest.cc b/...tion/wifi_data_provider_linux_unittest.cc → ...tion/wifi_data_provider_linux_unittest.cc
@@ -41,8 +41,7 @@ class GeolocationWifiDataProviderLinuxTest : public testing::Test {
     // Set an expectation so mock_network_manager_proxy_'s
     // CallMethodAndBlock() will use CreateNetworkManagerProxyResponse()
     // to return responses.
-    EXPECT_CALL(*mock_network_manager_proxy_.get(),
-                CallMethodAndBlock(_, _))
+    EXPECT_CALL(*mock_network_manager_proxy_.get(), CallMethodAndBlock(_, _))
         .WillRepeatedly(Invoke(this, &GeolocationWifiDataProviderLinuxTest::
                                          CreateNetworkManagerProxyResponse));
 

diff --git a/...cation/wifi_data_provider_win_unittest.cc → ...cation/wifi_data_provider_win_unittest.cc b/...cation/wifi_data_provider_win_unittest.cc → ...cation/wifi_data_provider_win_unittest.cc
@@ -5,8 +5,8 @@
 // Most logic for the platform wifi provider is now factored into
 // WifiDataProviderCommon and covered by it's unit tests.
 
-#include "base/message_loop/message_loop.h"
 #include "device/geolocation/wifi_data_provider_win.h"
+#include "base/message_loop/message_loop.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace device {

diff --git a/...olocation/wifi_polling_policy_unittest.cc → ...olocation/wifi_polling_policy_unittest.cc b/...olocation/wifi_polling_policy_unittest.cc → ...olocation/wifi_polling_policy_unittest.cc