Chrome Performance Monitor: MetricDetails refactor.

chrome/browser/performance_monitor/database_unittest.cc

 // Copyright (c) 2012 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 <string>
 #include <vector>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time.h"
 #include "chrome/browser/performance_monitor/database.h"
+#include "chrome/browser/performance_monitor/metric_details.h"
 #include "chrome/browser/performance_monitor/performance_monitor_util.h"
 #include "chrome/common/extensions/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using extensions::Extension;
 
 namespace performance_monitor {
 
 // A clock that increments every access. Great for testing.
 class TestingClock : public Database::Clock {
@@ skipping 62 matching lines @@
 
 class PerformanceMonitorDatabaseMetricTest : public ::testing::Test {
  protected:
   PerformanceMonitorDatabaseMetricTest() {
     clock_ = new TestingClock();
     file_util::CreateNewTempDirectory(FilePath::StringType(), &temp_path_);
     db_ = Database::Create(temp_path_);
     CHECK(db_.get());
     db_->set_clock(scoped_ptr<Database::Clock>(clock_));
     activity_ = std::string("A");
-    cpu_metric_ = std::string("CPU");
-    mem_metric_ = std::string("MEM");
-    fun_metric_ = std::string("FUN");
-    cpu_metric_details_ = MetricDetails(cpu_metric_, "Testing CPU metric.");
-    mem_metric_details_ = MetricDetails(mem_metric_, "Testing MEM metric.");
-    fun_metric_details_ = MetricDetails(fun_metric_, "Testing FUN metric.");
   }
 
   void SetUp() {
     ASSERT_TRUE(db_.get());
     PopulateDB();
   }
 
+  // TODO(mtytel): Once more metrics are added, populate the database with
+  // different metrics.
   void PopulateDB() {

[eaugusti, 2012/07/21 02:00:40]

I think that testing with only one metric type does not exercise enough code
paths. Could we do something to get another metric type in here?

[Matt Tytel, 2012/07/21 02:11:40]

Dern.. I deleted the patch set with the comment here.
Mitchell has a CL up that adds some metrics. I'm going to add the additional
tests when they go in and before this CL is through.
http://codereview.chromium.org/10656052/

[Matt Tytel, 2012/07/23 22:24:00]

Actually I just added a couple into mine.
Done.

-    db_->AddMetricDetails(cpu_metric_details_);
-    db_->AddMetricDetails(mem_metric_details_);
-    db_->AddMetricDetails(fun_metric_details_);
-    db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("50.5"));
-    db_->AddMetric(activity_, cpu_metric_, std::string("13.1"));
-    db_->AddMetric(kProcessChromeAggregate, mem_metric_, std::string("100000"));
-    db_->AddMetric(activity_, mem_metric_, std::string("20000"));
-    db_->AddMetric(kProcessChromeAggregate, fun_metric_, std::string("3.14"));
-    db_->AddMetric(activity_, fun_metric_, std::string("1.68"));
+    db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("50.5"));
+    db_->AddMetric(activity_, METRIC_SAMPLE, std::string("13.1"));
   }
 
   scoped_ptr<Database> db_;
   Database::Clock* clock_;
   FilePath temp_path_;
   std::string activity_;
-  std::string cpu_metric_;
-  std::string mem_metric_;
-  std::string fun_metric_;
-  MetricDetails cpu_metric_details_;
-  MetricDetails mem_metric_details_;
-  MetricDetails fun_metric_details_;
 };
 
 ////// PerformanceMonitorDatabaseSetupTests ////////////////////////////////////
 TEST(PerformanceMonitorDatabaseSetupTest, OpenClose) {
   FilePath alternate_path;
   file_util::CreateNewTempDirectory(FilePath::StringType(), &alternate_path);
   scoped_ptr<Database> db = Database::Create(alternate_path);
   ASSERT_TRUE(db.get());
   ASSERT_TRUE(db->Close());
 }
@@ skipping 137 matching lines @@
   EXPECT_TRUE(events[0]->data()->Equals(new_install_event->data()));
   EXPECT_TRUE(events[1]->data()->Equals(new_uninstall_event->data()));
   events = db_->GetEvents(
       EVENT_EXTENSION_INSTALL, start_time, end_time);
   ASSERT_EQ(1u, events.size());
   EXPECT_TRUE(events[0]->data()->Equals(new_install_event->data()));
 }
 
 ////// PerformanceMonitorDatabaseMetricTests ///////////////////////////////////
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetActiveMetrics) {
-  std::vector<MetricDetails> metrics =
+  std::vector<const MetricDetails*> metrics =
     db_->GetActiveMetrics(base::Time(), clock_->GetTime());
-  ASSERT_EQ(3u, metrics.size());
-  std::vector<MetricDetails>::iterator it;
-  for (it = metrics.begin(); it != metrics.end(); ++it) {
-    if (it->name == cpu_metric_details_.name)
-      EXPECT_EQ(it->description, cpu_metric_details_.description);
-    else if (it->name == mem_metric_details_.name)
-      EXPECT_EQ(it->description, mem_metric_details_.description);
-    else if (it->name == fun_metric_details_.name)
-      EXPECT_EQ(it->description, fun_metric_details_.description);
-    else
-      NOTREACHED();
-  }
+  ASSERT_EQ(1u, metrics.size());
+  EXPECT_EQ(GetMetricDetails(METRIC_SAMPLE), metrics[0]);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetState) {
   std::string key("version");
   std::string value("1.0.0.0.1");
   db_->AddStateValue(key, value);
   EXPECT_EQ(db_->GetStateValue(key), value);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStateOverride) {
   std::string key("version");
   std::string value_1("1.0.0.0.0");
   std::string value_2("1.0.0.0.1");
   db_->AddStateValue(key, value_1);
   db_->AddStateValue(key, value_2);
   EXPECT_EQ(db_->GetStateValue(key), value_2);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForActivityAndMetric) {
   Database::MetricInfoVector stats = db_->GetStatsForActivityAndMetric(
-      activity_, cpu_metric_, base::Time(), clock_->GetTime());
+      activity_, METRIC_SAMPLE, base::Time(), clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, cpu_metric_, std::string("18"));
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_,
+  db_->AddMetric(activity_, METRIC_SAMPLE, std::string("18"));
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_SAMPLE,
                                             before, clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(18, stats[0].value);
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_);
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_SAMPLE);
   ASSERT_EQ(2u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   EXPECT_EQ(18, stats[1].value);
-  stats = db_->GetStatsForActivityAndMetric(mem_metric_);
-  ASSERT_EQ(1u, stats.size());
-  EXPECT_EQ(100000, stats[0].value);
-  stats = db_->GetStatsForActivityAndMetric(activity_, cpu_metric_,
+  // stats = db_->GetStatsForActivityAndMetric(METRIC_NUMBER_OF_METRICS);

[eaugusti, 2012/07/21 02:00:40]

Why are these left in?

[Matt Tytel, 2012/07/21 02:11:40]

Because I'm careless. When Mitchell's patch goes through I'll fix this up.

[Matt Tytel, 2012/07/23 22:24:00]

Done.

+  // ASSERT_EQ(1u, stats.size());
+  // EXPECT_EQ(3, stats[0].value);
+  stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_SAMPLE,
                                             clock_->GetTime(),
                                             clock_->GetTime());
   EXPECT_TRUE(stats.empty());
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
   Database::MetricVectorMap stats_map = db_->GetStatsForMetricByActivity(
-      cpu_metric_, base::Time(), clock_->GetTime());
+      METRIC_SAMPLE, base::Time(), clock_->GetTime());
   ASSERT_EQ(2u, stats_map.size());
   linked_ptr<Database::MetricInfoVector> stats = stats_map[activity_];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(13.1, stats->at(0).value);
   stats = stats_map[kProcessChromeAggregate];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(50.5, stats->at(0).value);
-  stats_map = db_->GetStatsForMetricByActivity(cpu_metric_, clock_->GetTime(),
+  stats_map = db_->GetStatsForMetricByActivity(METRIC_SAMPLE, clock_->GetTime(),
                                                clock_->GetTime());
   EXPECT_EQ(0u, stats_map.size());
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_, cpu_metric_, std::string("18"));
-  stats_map = db_->GetStatsForMetricByActivity(cpu_metric_, before,
+  db_->AddMetric(activity_, METRIC_SAMPLE, std::string("18"));
+  stats_map = db_->GetStatsForMetricByActivity(METRIC_SAMPLE, before,
                                                clock_->GetTime());
   ASSERT_EQ(1u, stats_map.size());
   stats = stats_map[activity_];
   ASSERT_EQ(1u, stats->size());
   EXPECT_EQ(18, stats->at(0).value);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("3.4"));
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("3.4"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("21"));
   Database::MetricInfoVector stats =
-      db_->GetStatsForActivityAndMetric(cpu_metric_);
+      db_->GetStatsForActivityAndMetric(METRIC_SAMPLE);
   ASSERT_EQ(3u, stats.size());
   ASSERT_EQ(50.5, stats[0].value);
   ASSERT_EQ(3.4, stats[1].value);
   ASSERT_EQ(21, stats[2].value);
 }
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetRange) {
   base::Time start = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("3"));
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("9"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("3"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("9"));
   base::Time end = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate, cpu_metric_, std::string("21"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_SAMPLE, std::string("21"));
   Database::MetricInfoVector stats =
-      db_->GetStatsForActivityAndMetric(cpu_metric_, start, end);
+      db_->GetStatsForActivityAndMetric(METRIC_SAMPLE, start, end);
   ASSERT_EQ(2u, stats.size());
   ASSERT_EQ(3, stats[0].value);
   ASSERT_EQ(9, stats[1].value);
 }
 
 }  // namespace performance_monitor