Skeleton code of SparseHistogram

base/metrics/sparse_histogram_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 "base/logging.h"

[Ilya Sherman, 2012/08/04 01:18:35]

nit: What is this include needed for?

[kaiwang, 2012/08/08 03:59:33]

Done.

+#include "base/metrics/sparse_histogram.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {

[Ilya Sherman, 2012/08/04 01:18:35]

nit: Please leave a blank line after this line.

[kaiwang, 2012/08/08 03:59:33]

Done.

+class SparseHistogramTest : public testing::Test {
+ protected:
+  SparseHistogram* NewSparseHistogram(const std::string& name) {
+    return new SparseHistogram(name);

[Ilya Sherman, 2012/08/04 01:18:35]

Why is the test code using the constructor, rather than FactoryGet?  If you keep
this design, please don't leak the histogram from the test code -- there's no
need.  Also, please add a test that does use FactoryGet, and verify that the
flags are set correctly.

[kaiwang, 2012/08/08 03:59:33]

Deleted the newed histogram.

Will test FactoryGet when it's fully implemented

+  }
+};
+
+TEST_F(SparseHistogramTest, BasicTest) {
+  SparseHistogram* histogram = NewSparseHistogram("Sparse1");
+  SparseHistogram::SampleCounts sample;
+  histogram->SnapshotSample(&sample);
+
+  ASSERT_EQ(0u, sample.size());
+
+  histogram->Add(100);
+  histogram->SnapshotSample(&sample);
+  ASSERT_EQ(1u, sample.size());
+  EXPECT_EQ(1, sample[100]);
+
+  histogram->Add(100);
+  histogram->Add(101);
+  histogram->SnapshotSample(&sample);
+  ASSERT_EQ(2u, sample.size());
+  EXPECT_EQ(2, sample[100]);
+  EXPECT_EQ(1, sample[101]);
+}

[Ilya Sherman, 2012/08/04 01:18:35]

Since the SparseHistogram code uses locks, I would expect to see the test code
exercise those locks.

[kaiwang, 2012/08/08 03:59:33]

What's the common way in chromium code to test locks?

[Ilya Sherman, 2012/08/08 05:00:29]

I'm not sure if there's an especially common way; emailing chromium-dev might
give you a good answer there.  Offhand, though, I would try interacting with the
SparseHistogram from multiple threads, and ensure that the test passes under
TSan.

+
+}  // namespace base