ChromeOs Power Emulation Impl

chromeos/dbus/fake_power_manager_client_unittest.cc

+// 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 "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace {
+class TestObserver : public PowerManagerClient::Observer {
+ public:
+  TestObserver() : num_request_notify_observers_(0) {}

[Daniel Erat, 2015/07/09 15:41:48]

add an empty virtual d'tor:
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance

"Make your destructor virtual if necessary. If your class has virtual methods,
its destructor should be virtual."

(i think that clang tests for this when building)

[mozartalouis, 2015/07/14 20:01:28]

Done.

+
+  power_manager::PowerSupplyProperties& props() { return props_; }

[Daniel Erat, 2015/07/09 15:41:48]

both the return value and this method itself should be const:

const power_manager::PowerSupplyProperties& props() const { return props_; }

[mozartalouis, 2015/07/14 20:01:28]

Done.

+  const int num_request_notify_observers() {

[Daniel Erat, 2015/07/09 15:41:48]

this method should be const. the copy-by-value return value shouldn't be:

int num_request_notify_observers() const {

[mozartalouis, 2015/07/14 20:01:28]

Done.

+    return num_request_notify_observers_;
+  }
+
+  void PowerChanged(
+      const power_manager::PowerSupplyProperties& proto) override {
+    props_ = proto;
+    ++num_request_notify_observers_;
+  }
+
+ private:
+  int num_request_notify_observers_;

[Daniel Erat, 2015/07/09 15:41:47]

this should probably be num_power_changed_ instead

[mozartalouis, 2015/07/14 20:01:28]

Done.

+  power_manager::PowerSupplyProperties props_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+}  // namespace
+
+TEST(FakePowerManagerClientTest, UpdatePowerPropertiesTest) {
+  // Checking to verify when UpdatePowerProperties is called,
+  // |props_| values are updated.
+  FakePowerManagerClient client_;
+
+  client_.UpdatePowerProperties(
+      90, false, power_manager::PowerSupplyProperties_BatteryState_CHARGING,
+      power_manager::PowerSupplyProperties_ExternalPower_USB);
+
+  EXPECT_EQ(90, client_.props().battery_percent());
+  EXPECT_FALSE(client_.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_CHARGING,
+            client_.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_USB,
+            client_.props().external_power());
+};
+

[oshima, 2015/07/09 17:10:16]

can you also test if RequestStatusUpdate() works correctly?

[mozartalouis, 2015/07/14 20:01:28]

Done.

+TEST(FakePowerManagerClientTest, NotifyObserversTest) {
+  TestObserver test_obeserver;

[Daniel Erat, 2015/07/09 15:41:47]

this variable is misspelled throughout the function

[mozartalouis, 2015/07/14 20:01:28]

Done.

+  FakePowerManagerClient client_;

[Daniel Erat, 2015/07/09 15:41:48]

no trailing underscore here; those are just to denote class members

[mozartalouis, 2015/07/14 20:01:28]

Done.

+
+  // Test adding observer.
+  client_.AddObserver(&test_obeserver);
+  EXPECT_TRUE(client_.observers().HasObserver(&test_obeserver));

[Daniel Erat, 2015/07/09 15:41:48]

just call client.HasObserver()

[mozartalouis, 2015/07/14 20:01:28]

Done.

+
+  // Test if NotifyObservers sends the corrent values to ||.

[Daniel Erat, 2015/07/09 15:41:47]

sp: correct

did you mean |observer| instead of ||?

[mozartalouis, 2015/07/14 20:01:28]

Done.

+  client_.UpdatePowerProperties(
+      90, true, power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+      power_manager::PowerSupplyProperties_ExternalPower_DISCONNECTED);
+
+  EXPECT_EQ(90, test_obeserver.props().battery_percent());
+  EXPECT_TRUE(test_obeserver.props().is_calculating_battery_time());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_BatteryState_DISCHARGING,
+            test_obeserver.props().battery_state());
+  EXPECT_EQ(power_manager::PowerSupplyProperties_ExternalPower_DISCONNECTED,
+            test_obeserver.props().external_power());
+
+  // Check number of times NotifyObservers is called.
+  EXPECT_EQ(1, test_obeserver.num_request_notify_observers());
+
+  // Test removing observer
+  client_.RemoveObserver(&test_obeserver);
+  EXPECT_FALSE(client_.observers().HasObserver(&test_obeserver));

[Daniel Erat, 2015/07/09 15:41:47]

client.HasObserver()

[mozartalouis, 2015/07/14 20:01:28]

Done.

+};
+
+}  // namespace chromeos