ChromeOs Power Emulation Impl

chromeos/dbus/fake_power_manager_client_unittest.cc

Index: chromeos/dbus/fake_power_manager_client_unittest.cc
diff --git a/chromeos/dbus/fake_power_manager_client_unittest.cc b/chromeos/dbus/fake_power_manager_client_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d8f3e7e99dc4b19395697cb33cce0c9b10e820c1
--- /dev/null
+++ b/chromeos/dbus/fake_power_manager_client_unittest.cc
@@ -0,0 +1,139 @@
+// Copyright 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 "chromeos/dbus/fake_power_manager_client.h"
+
+#include "base/basictypes.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace {
+class TestObserver : public PowerManagerClient::Observer {
+ public:
+  TestObserver() : num_power_changed_(0) {}
+  ~TestObserver() override {}
+
+  const power_manager::PowerSupplyProperties& props() const { return props_; }
+  int num_power_changed() const { return num_power_changed_; }
+
+  void ClearProps() { props_.Clear(); }
+
+  void PowerChanged(
+      const power_manager::PowerSupplyProperties& proto) override {
+    props_ = proto;
+    ++num_power_changed_;
+  }
+
+ private:
+  int num_power_changed_;
+  power_manager::PowerSupplyProperties props_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+void SetTestProperties(power_manager::PowerSupplyProperties* props) {
+  props->set_battery_percent(85);
+  props->set_is_calculating_battery_time(true);
+  props->set_battery_state(
+      power_manager::PowerSupplyProperties_BatteryState_DISCHARGING);
+  props->set_external_power(
+      power_manager::PowerSupplyProperties_ExternalPower_USB);
+}
+
+}  // namespace
+
+TEST(FakePowerManagerClientTest, UpdatePowerPropertiesTest) {
+  // Checking to verify when UpdatePowerProperties is called,
+  // |props_| values are updated.
+  FakePowerManagerClient client;
+  power_manager::PowerSupplyProperties props;
+
+  SetTestProperties(&props);
+  client.UpdatePowerProperties(props);
+
+  EXPECT_EQ(85, client.props().battery_percent());

[Daniel Erat, 2015/07/15 19:05:47]

there are a bunch of constants (85, _DISCHARGING, etc.) hardcoded in multiple
places the source now. doing e.g.

  EXPECT_EQ(props.battery_percent(), client.props().battery_percent());
  EXPECT_EQ(props.battery_state(), client.props().battery_state());
  etc.

would be better.

[oshima, 2015/07/15 19:59:01]

I prefer tests to use constants or actual value rather than computed values.
If they get corrupted somehow, the above code won't be able to catch such error.

I'm fine to define constants for percent though.

[Daniel Erat, 2015/07/15 20:02:07]

constants are fine if they're also shared by SetTestProperties(); i was just
saying not to duplicate magic numbers and enum values all over the file.

[mozartalouis, 2015/07/15 21:26:38]

Done.

+  EXPECT_TRUE(client.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            client.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_USB,
+            client.props().external_power());
+
+  // Test if when the values are changed, the correct data is set in the
+  // FakePowerManagerClient.
+  props = client.props();
+  props.set_battery_percent(70);
+  client.UpdatePowerProperties(props);
+
+  EXPECT_EQ(70, client.props().battery_percent());
+  EXPECT_TRUE(client.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            client.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_USB,
+            client.props().external_power());
+};
+
+TEST(FakePowerManagerClientTest, NotifyObserversTest) {
+  FakePowerManagerClient client;
+  TestObserver test_observer;
+
+  // Test adding observer.
+  client.AddObserver(&test_observer);
+  EXPECT_TRUE(client.HasObserver(&test_observer));
+
+  // Test if NotifyObservers() sends the correct values to |observer|.
+  // Check number of times NotifyObservers() is called.
+  power_manager::PowerSupplyProperties props;
+  SetTestProperties(&props);
+  client.UpdatePowerProperties(props);
+
+  EXPECT_EQ(85, test_observer.props().battery_percent());
+  EXPECT_TRUE(test_observer.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            test_observer.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_USB,
+            test_observer.props().external_power());
+  EXPECT_EQ(1, test_observer.num_power_changed());
+
+  // Test if RequestStatusUpdate() will propergate the data to the observer.

[Daniel Erat, 2015/07/15 19:05:46]

sp: propagate

[mozartalouis, 2015/07/15 21:26:38]

Done.

+  // Check number of times NotifyObservers is called.
+  // RequestStatusUpdate posts to the current message loop. This is
+  // nessesary becuase we want to make sure that NotifyObservers() is
+  // called as a result of RequestStatusUpdate().

[Daniel Erat, 2015/07/15 19:05:47]

i don't think i understand why you need to post a task in the fake class instead
of just notifying observers synchronously.

[mozartalouis, 2015/07/15 21:26:38]

From the PowerStatusViewTest.Basic the test fails: 

[ RUN      ] PowerStatusViewTest.Basic
../../ash/system/chromeos/power/power_status_view_unittest.cc:60: Failure
Value of: IsPercentageVisible()
  Actual: true
Expected: false

this happens because the test expects the notification is called asynchronously,
which is the same on the real device.

I added a comment in the RequestStatusUpdate(). 

Please let me know if this can be synchronous and ill update the test code.

+  base::MessageLoopForUI message_loop;

[oshima, 2015/07/15 18:46:50]

can you use base::RunLoop instead?

[mozartalouis, 2015/07/15 21:26:38]

Done.

+  test_observer.ClearProps();
+  client.RequestStatusUpdate();
+  message_loop.RunUntilIdle();
+
+  EXPECT_EQ(85, test_observer.props().battery_percent());
+  EXPECT_TRUE(test_observer.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            test_observer.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_USB,
+            test_observer.props().external_power());
+  EXPECT_EQ(2, test_observer.num_power_changed());
+
+  // Check when values are changed, the correct values are propergated to the

[Daniel Erat, 2015/07/15 19:05:47]

sp: propagated

[mozartalouis, 2015/07/15 21:26:38]

Done.

+  // Obserever

[Daniel Erat, 2015/07/15 19:05:47]

sp: observer

[mozartalouis, 2015/07/15 21:26:38]

Done.

+  props = client.props();
+  props.set_battery_percent(90);
+  props.set_external_power(
+      power_manager::PowerSupplyProperties_ExternalPower_AC);
+  client.UpdatePowerProperties(props);
+
+  EXPECT_EQ(90, test_observer.props().battery_percent());
+  EXPECT_TRUE(test_observer.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            test_observer.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_AC,
+            test_observer.props().external_power());
+  EXPECT_EQ(3, test_observer.num_power_changed());
+
+  // Test removing observer.
+  client.RemoveObserver(&test_observer);
+  EXPECT_FALSE(client.HasObserver(&test_observer));
+};
+
+}  // namespace chromeos