Create a histogram serialization mechanism in components/metrics

components/metrics/metrics_utils_unittest_chromeos.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 "components/metrics/metrics_utils_chromeos.h"
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/strings/stringprintf.h"
+#include "components/metrics/crash_sample_chromeos.h"
+#include "components/metrics/histogram_sample_chromeos.h"
+#include "components/metrics/linearhistogram_sample_chromeos.h"
+#include "components/metrics/metric_sample_chromeos.h"
+#include "components/metrics/sparsehistogram_sample_chromeos.h"
+#include "components/metrics/useraction_sample_chromeos.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace metrics {
+namespace {
+
+using base::DeleteFile;
+using base::File;
+using base::FilePath;
+using base::GetFileSize;
+using base::StringPrintf;
+using MetricsUtils::ReadSample;
+using std::string;
+
+void AreEqHistogram(HistogramSample* a, HistogramSample* b) {
+  ASSERT_EQ(a->name(), b->name());

[Ben Chan, 2014/04/18 20:27:08]

ASSERT_ stops the test immediately, do you want EXPECT_ instead as you may still
want to check other values even if one mismatch is found

[bsimonnet, 2014/04/18 23:53:49]

yes ! :)

+  ASSERT_EQ(a->sample(), b->sample());
+  ASSERT_EQ(a->min(), b->min());
+  ASSERT_EQ(a->max(), b->max());
+  ASSERT_EQ(a->nbucket(), b->nbucket());
+}
+
+void AreEqCrash(CrashSample* a, CrashSample* b) {
+  ASSERT_EQ(a->name(), b->name());
+}
+
+void AreEqLinear(LinearHistogramSample* a, LinearHistogramSample* b) {
+  ASSERT_EQ(a->name(), b->name());
+  ASSERT_EQ(a->sample(), b->sample());
+  ASSERT_EQ(a->max(), b->max());
+}
+
+void AreEqSparse(SparseHistogramSample* a, SparseHistogramSample* b) {
+  ASSERT_EQ(a->name(), b->name());
+  ASSERT_EQ(a->sample(), b->sample());
+}
+
+void AreEqAction(UserActionSample* a, UserActionSample* b) {
+  ASSERT_EQ(a->name(), b->name());
+}
+
+void AreEq(MetricSample* a, MetricSample* b) {
+  ASSERT_TRUE(a);
+  ASSERT_TRUE(b);
+  MetricSample::SampleType type = a->type();
+  ASSERT_EQ(a->type(), b->type());
+  if (type == MetricSample::CRASH) {
+    AreEqCrash(reinterpret_cast<CrashSample*>(a),
+               reinterpret_cast<CrashSample*>(b));
+  } else if (type == MetricSample::HISTOGRAM) {
+    AreEqHistogram(reinterpret_cast<HistogramSample*>(a),
+                   reinterpret_cast<HistogramSample*>(b));
+  } else if (type == MetricSample::SPARSE_HISTOGRAM) {
+    AreEqSparse(reinterpret_cast<SparseHistogramSample*>(a),
+                reinterpret_cast<SparseHistogramSample*>(b));
+  } else if (type == MetricSample::LINEAR_HISTOGRAM) {
+    AreEqLinear(reinterpret_cast<LinearHistogramSample*>(a),
+                reinterpret_cast<LinearHistogramSample*>(b));
+  } else if (type == MetricSample::USER_ACTION) {
+    AreEqAction(reinterpret_cast<UserActionSample*>(a),
+                reinterpret_cast<UserActionSample*>(b));
+  } else {
+    DLOG(ERROR) << "could not recognize type";
+    FAIL();
+  }
+}
+
+class MetricsUtilsChromeOSTest : public testing::Test {
+ protected:
+  static const int buff_size = 1024;

[Ben Chan, 2014/04/18 20:27:08]

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

+  char* buffer;

[Ben Chan, 2014/04/18 20:27:08]

scoped_ptr<char[]> buffer;

+  std::string filename;

[Ben Chan, 2014/04/18 20:27:08]

you can drop std:: as you already have 'using std::string' above.

+  FilePath filepath;
+  MetricsUtilsChromeOSTest() {
+    filename = "/tmp/chromeossampletest";

[Ben Chan, 2014/04/18 20:27:08]

avoid hardcoding a temp file for test (and without cleaning up)

use base::ScopedTempDir to create a temp directory and then create a temp file
there

+    filepath = FilePath(filename);
+  }
+  virtual void SetUp() {
+    DeleteFile(filepath, false);
+    buffer = new char[buff_size];
+  }
+
+  void TestSerialization(MetricSample* sample) {
+    std::string serialized(sample->ToString());

[Ben Chan, 2014/04/18 20:27:08]

you can drop std:: as you already have 'using std::string' above.

+    ASSERT_EQ('\0', serialized[serialized.length() - 1]);
+    AreEq(sample, MetricsUtils::ReadSample(serialized));
+  }
+};
+
+TEST_F(MetricsUtilsChromeOSTest, CrashSerializeTest) {
+  CrashSample crash("test");
+  TestSerialization(&crash);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, HistogramSerializeTest) {
+  HistogramSample hist("myhist", 13, 1, 100, 10);
+  TestSerialization(&hist);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, LinearSerializeTest) {
+  LinearHistogramSample lhist("linearhist", 12, 30);
+  TestSerialization(&lhist);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, SparseSerializeTest) {
+  SparseHistogramSample shist("mysparse", 30);
+  TestSerialization(&shist);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, UserActionSerializeTest) {
+  TestSerialization(new UserActionSample("myaction"));
+}
+
+TEST_F(MetricsUtilsChromeOSTest, IllegalNameAreFilteredTest) {
+  SparseHistogramSample sample1("no space", 10);
+  LinearHistogramSample sample2(StringPrintf("here%cbhe", '\0'), 1, 3);
+  EXPECT_FALSE(MetricsUtils::WriteMetricToFile(sample1, filename));
+  EXPECT_FALSE(MetricsUtils::WriteMetricToFile(sample2, filename));
+  int64 size = GetFileSize(filepath, &size);
+
+  EXPECT_EQ(0, size);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, BadInputIsCaughtTest) {
+  std::string input(StringPrintf("sparsehistogram%cname foo%c", '\0', '\0'));

[Ben Chan, 2014/04/18 20:27:08]

you can drop std:: as you already have 'using std::string' above.

+  EXPECT_EQ(NULL, SparseHistogramSample::ReadSparseHistogram(input));
+}
+
+TEST_F(MetricsUtilsChromeOSTest, MessageSeparatedByZero) {
+  CrashSample crash("crash");
+
+  MetricsUtils::WriteMetricToFile(crash, filename);
+  int64 size;

[Ben Chan, 2014/04/18 20:27:08]

if the compiler yells, you may need to initialize |size| to 0

+  GetFileSize(filepath, &size);

[Ben Chan, 2014/04/18 20:27:08]

ASSERT_TRUE(GetFileSize(filepath, &size));

+  ASSERT_EQ(size, 16);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, MessagesTooLongAreDiscardedTest) {
+  // Creates a message that is bigger than the maximum allowed size.
+  // As we are adding extra character (crash, \0s, etc), if the name is
+  // kMessageMaxLength long, it will be too long.
+  char c = 'c';
+  string name = "";
+  for (int i = 0; i < MetricsUtils::kMessageMaxLength; i++) {
+    name = name + c;
+  }

[Ben Chan, 2014/04/18 20:27:08]

you can simply do:

  string name(MetricsUtils::kMessageMaxLength, 'c');

+
+  CrashSample crash(name);
+  EXPECT_FALSE(MetricsUtils::WriteMetricToFile(crash, filename));
+  int64 size;
+  GetFileSize(filepath, &size);

[Ben Chan, 2014/04/18 20:27:08]

ditto

+  EXPECT_EQ(0, size);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, ReadLongMessageTest) {
+  File f(filepath, File::FLAG_OPEN_ALWAYS | File::FLAG_APPEND);

[Ben Chan, 2014/04/18 20:27:08]

nit: |f| feels like a bad variable

+  char c = 'c';
+  string message = "";
+  for (int i = 0; i < MetricsUtils::kMessageMaxLength + 1; i++) {
+    message = message + c;

[Ben Chan, 2014/04/18 20:27:08]

ditto

+  }
+
+  int32_t message_size = message.length() + sizeof(int32_t);
+  f.WriteAtCurrentPos(reinterpret_cast<char*>(&message_size),

[Ben Chan, 2014/04/18 20:27:08]

const char*

+                      sizeof(message_size));
+  f.WriteAtCurrentPos(message.c_str(), message.length());
+  f.Close();
+
+  CrashSample crash("test");
+  MetricsUtils::WriteMetricToFile(crash, filename);
+
+  ScopedVector<MetricSample> samples;
+  MetricsUtils::ReadAndTruncateMetricsFromFile(filename, &samples);
+  ASSERT_EQ(size_t(1), samples.size());
+  AreEq(&crash, samples[0]);
+}
+
+TEST_F(MetricsUtilsChromeOSTest, WriteReadTest) {
+  HistogramSample hist("myhist", 1, 2, 3, 4);
+  CrashSample crash("crash");
+  LinearHistogramSample lhist("linear", 1, 10);
+  SparseHistogramSample shist("mysparse", 30);
+  UserActionSample action("myaction");
+
+  MetricsUtils::WriteMetricToFile(hist, filename);
+  MetricsUtils::WriteMetricToFile(crash, filename);
+  MetricsUtils::WriteMetricToFile(lhist, filename);
+  MetricsUtils::WriteMetricToFile(shist, filename);
+  MetricsUtils::WriteMetricToFile(action, filename);
+  ScopedVector<MetricSample> vect;
+  MetricsUtils::ReadAndTruncateMetricsFromFile(filename, &vect);
+  ASSERT_EQ(vect.size(), size_t(5));
+  AreEq(&hist, vect[0]);
+  AreEq(&crash, vect[1]);
+  AreEq(&lhist, vect[2]);
+  AreEq(&shist, vect[3]);
+  AreEq(&action, vect[4]);
+
+  int64 size;
+  GetFileSize(filepath, &size);

[Ben Chan, 2014/04/18 20:27:08]

ditto

+  ASSERT_EQ(0, size);
+}
+
+}  // namespace
+}  // namespace metrics