Add HistogramTester::GetAllSamples.

base/test/histogram_tester_unittest.cc

 // Copyright 2014 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 "base/test/histogram_tester.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/histogram_samples.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 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";
 
 typedef testing::Test HistogramTesterTest;
 
 TEST_F(HistogramTesterTest, Scope) {
   // Record a histogram before the creation of the recorder.
   UMA_HISTOGRAM_BOOLEAN(kHistogram1, true);
@@ skipping 47 matching lines @@
 
   HistogramTester tester;
   UMA_HISTOGRAM_COUNTS_100(kHistogram4, 3);
 
   tester.ExpectBucketCount(kHistogram4, 2, 0);
   tester.ExpectBucketCount(kHistogram4, 3, 1);
 
   tester.ExpectTotalCount(kHistogram4, 1);
 }
 
+TEST_F(HistogramTesterTest, TestGetAllSamples) {
+  HistogramTester tester;
+  UMA_HISTOGRAM_COUNTS_100(kHistogram2, 2);
+  UMA_HISTOGRAM_COUNTS_100(kHistogram2, 3);
+  UMA_HISTOGRAM_COUNTS_100(kHistogram2, 3);
+  UMA_HISTOGRAM_COUNTS_100(kHistogram2, 5);
+
+  EXPECT_THAT(tester.GetAllSamples(kHistogram2),
+              ElementsAre(std::make_pair(2, 1), std::make_pair(3, 2),
+                          std::make_pair(5, 1)));

[ncarter (slow), 2015/06/24 17:28:44]

I patched this in and played around with it. I think using EXPECT_THAT is just
as good as the flattened text representation I came up with in the other CL.
Using the text representation arguably has less of a learning curve and gives
very slightly more readable errors, but on the other hand, it introduces a
convention of its own that has to be explained.

If the other reviewers are, like me, okay with unifying behind the EXPECT_THAT
approach, my CL could become this:

  map<string, int> GetTotalCountsForPrefix(string name_prefix);

with example usage:

  EXPECT_THAT(tester.GetTotalCountsForPrefix("MyMetric."),
              ElementsAre(std::make_pair("MyMetric.A", 1), 
                          std::make_pair("MyMetric.A", 2)));

 or for more complex usage:

  std::map<std::string, int> expected_counts;
  expected_counts["MyMetric.A"] = 1;
  expected_counts["MyMetric.B"] = 1;
  EXPECT_THAT(tester.GetTotalCountsForPrefix("MyMetric."),
              ContainerEq(expected_counts));

And if, after using the *ForPrefix to determine which metrics had samples, you
wanted to inspect buckets, you'd use GetAllSamples for that, one metric at a
time.

I think these approaches would then fit together nicely (and let us hoist the
EXPECTS back up into the tests, which is a common purpose of both proposals). I
also think we ought to strive to eliminate the Expect* methods altogether so
that there aren't multiple redundant ways to do this.

[twifkak, 2015/06/25 20:21:38]

Okay, sounds good!

Do you want me to implement GetTotalCountsForPrefix, or modify your CL per this
API?

+}
+
 }  // namespace base