Unit test for Data Saver prompt notification

chrome/browser/chromeos/status/data_promo_notification_unittest.cc

+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/status/data_promo_notification.h"
+
+#include "base/command_line.h"
+#include "base/run_loop.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_service_client.h"
+#include "chromeos/login/login_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "testing/platform_test.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+#include "ui/chromeos/network/network_connect.h"
+#include "ui/message_center/message_center.h"
+
+namespace {
+
+const char kCellularServicePath[] = "/service/cellular1";
+
+class NetworkConnectTestDelegate : public ui::NetworkConnect::Delegate {
+ public:
+  NetworkConnectTestDelegate() {}
+  ~NetworkConnectTestDelegate() override {}
+
+  void ShowNetworkConfigure(const std::string& network_id) override {}
+  void ShowNetworkSettingsForGuid(const std::string& network_id) override {}
+  bool ShowEnrollNetwork(const std::string& network_id) override {
+    return false;
+  }
+  void ShowMobileSimDialog() override {}
+  void ShowMobileSetupDialog(const std::string& service_path) override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NetworkConnectTestDelegate);
+};
+
+}  // namespace
+
+namespace chromeos {
+namespace test {
+
+class DataPromoNotificationTest : public testing::Test {
+ public:
+  DataPromoNotificationTest() {}
+  ~DataPromoNotificationTest() override {}
+
+  void SetUp() override {
+    testing::Test::SetUp();
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnableDataSaverPrompt);
+    DBusThreadManager::Initialize();
+    NetworkHandler::Initialize();
+    data_promo_notification_.reset(new DataPromoNotification);
+    SetupUser();
+    SetupNetworkShillState();
+    message_center::MessageCenter::Initialize();
+    base::RunLoop().RunUntilIdle();
+    network_connect_delegate_.reset(new NetworkConnectTestDelegate);
+    ui::NetworkConnect::Initialize(network_connect_delegate_.get());
+  }
+
+  void TearDown() override {
+    ui::NetworkConnect::Shutdown();
+    network_connect_delegate_.reset();
+    message_center::MessageCenter::Shutdown();
+    LoginState::Shutdown();
+    profile_manager_.reset();
+    user_manager_enabler_.reset();
+    data_promo_notification_.reset();
+    NetworkHandler::Shutdown();
+    DBusThreadManager::Shutdown();
+    testing::Test::TearDown();
+  }
+
+ protected:
+  void SetupUser() {
+    FakeChromeUserManager* user_manager = new FakeChromeUserManager();

[Greg Levin, 2015/04/27 23:03:08]

Does this need a matching delete somewhere?  I copied this from
network_portal_detector_impl_unittest.cc, which doesn't seem to have one.

[stevenjb, 2015/04/28 22:32:59]

No, because we are passing ownership to user_manager_enabler_, but to make that
clear this should be:

scoped_ptr<UserManager> user_manager(new FakeChromeUserManager());
user_manager_enabler_.reset(new
ScopedUserManagerEnabler(user_manager.release()));

Ideally ScopedUserManagerEnabler would take a scoped_ptr<UserManager> to make
the ownership passing explicit, and we would use Pass() instead of release().

[Greg Levin, 2015/04/29 14:46:09]

Ok, got it, thanks!  I rearranged a couple lines to accommodate.

+    user_manager_enabler_.reset(new ScopedUserManagerEnabler(user_manager));
+
+    const char kTestUserName[] = "test-user@example.com";
+    user_manager->AddUser(kTestUserName);
+    user_manager->LoginUser(kTestUserName);
+
+    profile_manager_.reset(
+        new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+    ASSERT_TRUE(profile_manager_->SetUp());
+    profile_manager_->SetLoggedIn(true);
+    ASSERT_TRUE(user_manager::UserManager::Get()->GetPrimaryUser());
+
+    LoginState::Initialize();
+    LoginState::Get()->SetLoggedInState(LoginState::LOGGED_IN_ACTIVE,
+                                        LoginState::LOGGED_IN_USER_REGULAR);
+  }
+
+  void SetupNetworkShillState() {
+    base::RunLoop().RunUntilIdle();
+
+    // Create a cellular device with provider.
+    ShillDeviceClient::TestInterface* device_test =
+        DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface();
+    device_test->ClearDevices();
+    const char kCellularDevicePath[] = "/device/stub_cellular_device1";

[Greg Levin, 2015/04/27 23:03:08]

1) Do we prefer "const char" or "const std::string"?

2) For const strings that only get used in one function, do we prefer defining
them at the top of the file, or right before their first use in the function?

For each question, I’ve seen both used in unittest files.

[stevenjb, 2015/04/28 22:32:59]

1) Outside of the scope of a function or class we need to use const char[] to
avoid the overhead of global constructors that have to occur on application
startup. Inside a function const std::string is fine (but you'll see both people
are used to the global case).
2) If it's only used once you can just inline it same as e.g.
"stub_cellular_device1", particularly in a unit test. If it's used more than
once it's probably better at the top of the file.

[Greg Levin, 2015/04/29 14:46:09]

Done.

+    device_test->AddDevice(kCellularDevicePath, shill::kTypeCellular,
+                           "stub_cellular_device1");
+    base::DictionaryValue home_provider;
+    home_provider.SetString("name", "Cellular1_Provider");
+    home_provider.SetString("country", "us");
+    device_test->SetDeviceProperty(kCellularDevicePath,
+                                   shill::kHomeProviderProperty, home_provider);
+
+    // Create a cellular network and activate it.
+    ShillServiceClient::TestInterface* service_test =
+        DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
+    service_test->ClearServices();
+    service_test->AddService(kCellularServicePath, "cellular1_guid",
+                             "cellular1" /* name */, shill::kTypeCellular,
+                             "activated", true /* visible */);
+    service_test->SetServiceProperty(
+        kCellularServicePath, shill::kActivationStateProperty,
+        base::StringValue(shill::kActivationStateActivated));
+    service_test->SetServiceProperty(kCellularServicePath,
+                                     shill::kConnectableProperty,
+                                     base::FundamentalValue(true));
+  }
+
+  scoped_ptr<DataPromoNotification> data_promo_notification_;
+  scoped_ptr<NetworkConnectTestDelegate> network_connect_delegate_;
+  scoped_ptr<ScopedUserManagerEnabler> user_manager_enabler_;
+  scoped_ptr<TestingProfileManager> profile_manager_;
+  base::MessageLoop message_loop_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DataPromoNotificationTest);
+};
+
+TEST_F(DataPromoNotificationTest, DataSaverNotification) {
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+
+  // Network setup shouldn't be enough to activate notification.
+  const char kNotificationId[] = "chrome://settings/internet/data_saver";
+  EXPECT_FALSE(message_center->FindVisibleNotificationById(kNotificationId));
+
+  ui::NetworkConnect::Get()->ConnectToNetwork(kCellularServicePath);
+  base::RunLoop().RunUntilIdle();
+  // Connecting to cellular network (which here makes it the default network)
+  // should trigger the Data Saver notification.
+  EXPECT_TRUE(message_center->FindVisibleNotificationById(kNotificationId));
+}
+
+}  // namespace test
+}  // namespace chromeos