chromeos: Add CPU temperature reader

chromeos/system/cpu_temperature_reader_unittest.cc

+// Copyright 2017 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/system/cpu_temperature_reader.h"
+
+#include <algorithm>
+#include <memory>
+#include <string>
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace system {
+
+class CPUTemperatureReaderTest : public ::testing::Test {
+ public:
+  ~CPUTemperatureReaderTest() override {}
+
+  void SetUp() override {

[Daniel Erat, 2017/04/21 21:01:20]

put things in c'tor and d'tor instead of SetUp when possible:
https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#shoul...

[Simon Que, 2017/04/24 13:59:17]

Done.

+    dir_.reset(new base::ScopedTempDir);
+    CHECK(dir_->CreateUniqueTempDir());
+    hwmon_path_ = dir_->GetPath();
+    reader_.set_hwmon_dir_for_test(hwmon_path_.MaybeAsASCII());
+  }
+
+  void TearDown() override { dir_.reset(); }

[Daniel Erat, 2017/04/21 21:01:20]

why reset this? the d'tor will already destroy it for you

[Simon Que, 2017/04/24 13:59:17]

Done.

+
+ protected:
+  using CPUTemperatureInfo = CPUTemperatureReader::CPUTemperatureInfo;
+
+  // Creates a subdirectory in |hwmon_path_| with name |name|. Returns the full
+  // path of the new subdirectory.
+  base::FilePath CreateHwmonSubdir(const std::string& name) {
+    base::FilePath subdir_path = hwmon_path_.Append(name);
+    CHECK(base::CreateDirectory(subdir_path));
+    return subdir_path;
+  }
+
+  // Creates a file at |path| containing data |contents|. Returns true if it was
+  // successfully created and filled with |contents|.
+  bool CreateFileWithContents(const base::FilePath& path,

[Daniel Erat, 2017/04/21 21:01:20]

you never expect this to fail, right? return void and either ASSERT_EQ or
CHECK_EQ within it so the callers don't need to verify that it suceeded

[Simon Que, 2017/04/24 13:59:17]

Done.

+                              const std::string& contents) {
+    return WriteFile(path, contents.data(), contents.size()) ==
+           static_cast<int>(contents.size());
+  }
+
+  // Creates a temporary dir to act as the hwmon directory passed to |reader_|.
+  std::unique_ptr<base::ScopedTempDir> dir_;

[Daniel Erat, 2017/04/21 21:01:20]

why does this need to be in a unique_ptr?

[Simon Que, 2017/04/24 13:59:17]

Done.

+
+  // Path of the temporary dir created by |dir_|.
+  base::FilePath hwmon_path_;
+
+  // Instance of the class under test
+  CPUTemperatureReader reader_;
+};
+
+TEST_F(CPUTemperatureReaderTest, EmptyDir) {
+  auto subdir_empty = CreateHwmonSubdir("hwmon0");

[Daniel Erat, 2017/04/21 21:01:20]

use explicit type rather than 'auto' since the type isn't obvious from the
context; ditto below

[Simon Que, 2017/04/24 13:59:17]

Done.

+  auto subdir_not_temp = CreateHwmonSubdir("hwmon1");
+  EXPECT_TRUE(CreateFileWithContents(subdir_not_temp.Append("not_cpu_temp"),
+                                     "garbage"));
+
+  EXPECT_EQ(0U, reader_.GetCPUTemperatures().size());
+}
+
+TEST_F(CPUTemperatureReaderTest, SingleDir) {
+  auto subdir = CreateHwmonSubdir("hwmon0");
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("temp1_input"), "10000\n"));
+
+  auto cpu_temp_readings = reader_.GetCPUTemperatures();

[Daniel Erat, 2017/04/21 21:01:20]

use explicit types instead of 'auto'

[Simon Que, 2017/04/24 13:59:17]

Done.

+
+  ASSERT_EQ(1U, cpu_temp_readings.size());
+  EXPECT_EQ(10.0f, cpu_temp_readings[0].temp_celsius);
+  EXPECT_EQ(subdir.Append("temp1_label").value(), cpu_temp_readings[0].label);
+}
+
+TEST_F(CPUTemperatureReaderTest, SingleDirWithLabel) {
+  auto subdir = CreateHwmonSubdir("hwmon0");
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("temp2_input"), "20000\n"));
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("temp2_label"), "t2\n"));
+
+  auto cpu_temp_readings = reader_.GetCPUTemperatures();
+
+  ASSERT_EQ(1U, cpu_temp_readings.size());
+  EXPECT_EQ(20.0f, cpu_temp_readings[0].temp_celsius);
+  EXPECT_EQ("t2", cpu_temp_readings[0].label);
+}
+
+TEST_F(CPUTemperatureReaderTest, SingleDirWithName) {
+  auto subdir = CreateHwmonSubdir("hwmon0");
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("temp3_input"), "30000\n"));
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("temp3_label"), "\n"));
+  EXPECT_TRUE(CreateFileWithContents(subdir.Append("name"), "t3\n"));
+
+  auto cpu_temp_readings = reader_.GetCPUTemperatures();
+
+  ASSERT_EQ(1U, cpu_temp_readings.size());
+  EXPECT_EQ(30.0f, cpu_temp_readings[0].temp_celsius);
+  EXPECT_EQ("t3", cpu_temp_readings[0].label);
+}
+
+TEST_F(CPUTemperatureReaderTest, MultipleDirs) {
+  auto subdir0 = CreateHwmonSubdir("hwmon0");
+  EXPECT_TRUE(CreateFileWithContents(subdir0.Append("temp1_input"), "10000\n"));
+
+  auto subdir1 = CreateHwmonSubdir("hwmon1");
+  EXPECT_TRUE(CreateFileWithContents(subdir1.Append("temp2_input"), "20000\n"));
+  EXPECT_TRUE(CreateFileWithContents(subdir1.Append("temp2_label"), "t2\n"));
+
+  // This should not result in a CPU temperature reading.
+  auto subdir2 = CreateHwmonSubdir("hwmon2");
+  EXPECT_TRUE(
+      CreateFileWithContents(subdir2.Append("not_cpu_temp"), "garbage"));
+
+  auto subdir3 = CreateHwmonSubdir("hwmon3");
+  EXPECT_TRUE(CreateFileWithContents(subdir3.Append("temp3_input"), "30000\n"));
+  EXPECT_TRUE(CreateFileWithContents(subdir3.Append("temp3_label"), "t3\n"));
+
+  auto cpu_temp_readings = reader_.GetCPUTemperatures();
+
+  // The order in which these directories were read is not guaranteed. Sort them
+  // first.
+  std::sort(cpu_temp_readings.begin(), cpu_temp_readings.end());
+
+  ASSERT_EQ(3U, cpu_temp_readings.size());
+  EXPECT_EQ(10.0f, cpu_temp_readings[0].temp_celsius);
+  EXPECT_EQ(subdir0.Append("temp1_label").value(), cpu_temp_readings[0].label);
+  EXPECT_EQ(20.0f, cpu_temp_readings[1].temp_celsius);
+  EXPECT_EQ("t2", cpu_temp_readings[1].label);
+  EXPECT_EQ(30.0f, cpu_temp_readings[2].temp_celsius);
+  EXPECT_EQ("t3", cpu_temp_readings[2].label);
+}
+
+}  // namespace system
+}  // namespace chromeos