Chrome Performance Monitor: MetricDetails refactor.

chrome/browser/performance_monitor/database_unittest.cc

Index: chrome/browser/performance_monitor/database_unittest.cc
diff --git a/chrome/browser/performance_monitor/database_unittest.cc b/chrome/browser/performance_monitor/database_unittest.cc
index fb919611087f6a19a84ccfa7abdcfd51386cdbbb..e08ab2998c0ab63e563723f90f6a3af3026f9bb3 100644
--- a/chrome/browser/performance_monitor/database_unittest.cc
+++ b/chrome/browser/performance_monitor/database_unittest.cc
@@ -10,6 +10,7 @@
 #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"
@@ -92,12 +93,6 @@ class PerformanceMonitorDatabaseMetricTest : public ::testing::Test {
     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() {
@@ -105,28 +100,17 @@ class PerformanceMonitorDatabaseMetricTest : public ::testing::Test {
     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 ////////////////////////////////////
@@ -284,20 +268,10 @@ TEST_F(PerformanceMonitorDatabaseEventTest, GetEventsTimeRange) {
 
 ////// 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) {
@@ -318,23 +292,23 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStateOverride) {
 
 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());
@@ -342,7 +316,7 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForActivityAndMetric) {
 
 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());
@@ -350,12 +324,12 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
   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_];
@@ -364,10 +338,10 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetStatsForMetricByActivity) {
 }
 
 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);
@@ -376,12 +350,12 @@ TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
 
 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);