HistogramTester::GetHistogramSamplesSinceCreation never returns null

base/test/histogram_tester_unittest.cc

Index: base/test/histogram_tester_unittest.cc
diff --git a/base/test/histogram_tester_unittest.cc b/base/test/histogram_tester_unittest.cc
index f8755279c5ac749612d95b0dacdd89bd66deeb04..8452013f036c7b125deb9076d0864ca111c0409b 100644
--- a/base/test/histogram_tester_unittest.cc
+++ b/base/test/histogram_tester_unittest.cc
@@ -14,11 +14,11 @@ namespace base {
 
 using ::testing::ElementsAre;
 
-const std::string kHistogram1 = "Test1";
-const std::string kHistogram2 = "Test2";
-const std::string kHistogram3 = "Test3";
-const std::string kHistogram4 = "Test4";
-const std::string kHistogram5 = "Test5";
+static const char kHistogram1[] = "Test1";

[vabr (Chromium), 2015/08/03 07:33:37]

@thestig -- Here I followed you request literally, marking the constants as
static.

However, unlike on line 47, where "static" means that the constant is allocated
outside of the stack and spans the whole program execution, here it means that
the constants are not visible outside of this file (while non-stack allocation
and whole-execution-lifespan are implicit).

If I were writing the code as new, I would use anonymous namespace instead of
"static" here (and also move "namespace base" just before line 23).

I cannot assume your approval for that, so I will keep it as you literally told
me in this CL. But if you want me to do any follow-ups, I'll be happy to, just
let me know.

[Lei Zhang, 2015/08/03 18:00:30]

An anonymous namespace would have been fine. I should have been more clear.

+static const char kHistogram2[] = "Test2";
+static const char kHistogram3[] = "Test3";
+static const char kHistogram4[] = "Test4";
+static const char kHistogram5[] = "Test5";
 
 typedef testing::Test HistogramTesterTest;
 
@@ -29,19 +29,32 @@ TEST_F(HistogramTesterTest, Scope) {
   HistogramTester tester;
 
   // Verify that no histogram is recorded.
-  scoped_ptr<HistogramSamples> samples(
-      tester.GetHistogramSamplesSinceCreation(kHistogram1));
-  EXPECT_FALSE(samples);
+  tester.ExpectTotalCount(kHistogram1, 0);
 
   // Record a histogram after the creation of the recorder.
   UMA_HISTOGRAM_BOOLEAN(kHistogram1, true);
 
   // Verify that one histogram is recorded.
-  samples = tester.GetHistogramSamplesSinceCreation(kHistogram1);
+  scoped_ptr<HistogramSamples> samples(
+      tester.GetHistogramSamplesSinceCreation(kHistogram1));
   EXPECT_TRUE(samples);
   EXPECT_EQ(1, samples->TotalCount());
 }
 
+TEST_F(HistogramTesterTest, GetHistogramSamplesSinceCreationNotNull) {
+  // Chose the histogram name uniquely, to ensure nothing was recorded for it so
+  // far.
+  static const char kHistogram[] =
+      "GetHistogramSamplesSinceCreationNotNullHistogram";
+  HistogramTester tester;
+
+  // Verify that the returned samples are empty but not null.
+  scoped_ptr<HistogramSamples> samples(
+      tester.GetHistogramSamplesSinceCreation(kHistogram1));
+  EXPECT_TRUE(samples);
+  tester.ExpectTotalCount(kHistogram, 0);
+}
+
 TEST_F(HistogramTesterTest, TestUniqueSample) {
   HistogramTester tester;