[Offline Pages] Create a event/activity logger

components/offline_pages/offline_page_model_event_logger_unittest.cc

Index: components/offline_pages/offline_page_model_event_logger_unittest.cc
diff --git a/components/offline_pages/offline_page_model_event_logger_unittest.cc b/components/offline_pages/offline_page_model_event_logger_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8704e1802ada7af626ac12efbd8bc93c00c0ca86
--- /dev/null
+++ b/components/offline_pages/offline_page_model_event_logger_unittest.cc
@@ -0,0 +1,56 @@
+// Copyright 2016 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/offline_pages/offline_page_model_event_logger.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace offline_internals {

[fgorski, 2016/06/23 16:34:07]

same comments apply to this file as to the other one.

(namespace, TEST(), magic number). Please apply here.

[chili, 2016/06/24 02:45:52]

Done.

+
+namespace {
+
+class OfflinePageModelEventLoggerTest : public ::testing::Test {
+ public:
+  OfflinePageModelEventLoggerTest() {}
+  ~OfflinePageModelEventLoggerTest() {}
+};
+
+TEST_F(OfflinePageModelEventLoggerTest, RecordsWhenLoggingIsOn) {
+  OfflinePageModelEventLogger logger;
+  std::vector<std::string> log;
+
+  logger.SetIsLogging(true);
+  logger.RecordStoreCleared();
+  logger.GetLogs(log);
+
+  EXPECT_EQ(1, log.size());

[Pete Williamson, 2016/06/24 00:38:32]

It would be nice to check the contents of the log string also if you can (I made
the same comment in the Request Coordinator file).

[chili, 2016/06/24 02:45:52]

I had this debate with myself.  I ended up not checking the contents of the log
string because I think a change in how the log string is formatted shouldn't
result in a change of the test.  However, I do think the test as is feels kind
of empty, so let me know what you think :)

+}
+
+TEST_F(OfflinePageModelEventLoggerTest, DoesNotRecordWhenLoggingIsOff) {
+  OfflinePageModelEventLogger logger;
+  std::vector<std::string> log;
+
+  logger.SetIsLogging(false);
+  logger.RecordStoreCleared();
+  logger.GetLogs(log);
+
+  EXPECT_EQ(0, log.size());
+}
+
+TEST_F(OfflinePageModelEventLoggerTest, DoesNotExceedMaxSize) {
+  OfflinePageModelEventLogger logger;
+  std::vector<std::string> log;
+
+  logger.SetIsLogging(true);
+  for (int i = 0; i < 21; i++) {

[dewittj, 2016/06/23 22:19:02]

please document maximum size in code like

const int kMaxLogSize = 20;
for (int i = 0; i < kMaxLogSize + 1; ++i) {
...
}

Also, small C++ nit: Many Chromies prefer that you write preincrement rather
than postincrement operators in for loops.

[chili, 2016/06/24 02:45:52]

:(     .... but i like postincrement >"<

Changed

+    logger.RecordStoreCleared();
+  }
+  logger.GetLogs(log);
+
+  EXPECT_EQ(20, log.size());
+}
+
+} // namespace
+
+} // namespace offline_internals