Add metrics library tests. Some metrics daemon API cleanup.
Refactor the metrics daemon API a little so that we don't need
to link in libmetrics into the daemon test binary.
Review URL: http://codereview.chromium.org/2079007
diff --git a/metrics/Makefile b/metrics/Makefile
index 5ce0caf..e0c3741 100644
--- a/metrics/Makefile
+++ b/metrics/Makefile
@@ -15,11 +15,15 @@
DAEMON_TEST = metrics_daemon_test
LIB = libmetrics.a
SHAREDLIB = libmetrics.so
+LIB_TEST = metrics_library_test
CLIENT_OBJS = \
metrics_client.o
LIB_OBJS = \
metrics_library.o
+TESTLIB_OBJS = \
+ metrics_library.o \
+ metrics_library_test.o
DAEMON_OBJS = \
metrics_daemon.o \
metrics_daemon_main.o
@@ -27,12 +31,13 @@
metrics_daemon.o \
metrics_daemon_test.o
-DAEMON_LDFLAGS = $(LDCONFIG) -lrt -lbase -lpthread -lgflags
+DAEMON_LDFLAGS = $(LDFLAGS) $(LDCONFIG) -lrt -lbase -lpthread -lgflags
TESTDAEMON_LIBS = -lgmock -lgtest
+TESTLIB_LIBS = -lgtest -lbase -lrt -lpthread
all: $(LIB) $(SHAREDLIB) $(CLIENT) $(DAEMON)
-tests: $(DAEMON_TEST)
+tests: $(DAEMON_TEST) $(LIB_TEST)
$(CLIENT): $(CLIENT_OBJS) $(SHAREDLIB)
$(CXX) $(LDFLAGS) $^ -o $@
@@ -40,7 +45,7 @@
$(DAEMON): $(DAEMON_OBJS) $(SHAREDLIB)
$(CXX) -o $@ $^ $(DAEMON_LDFLAGS)
-$(DAEMON_TEST): $(TESTDAEMON_OBJS) $(SHAREDLIB)
+$(DAEMON_TEST): $(TESTDAEMON_OBJS)
$(CXX) -o $@ $^ $(DAEMON_LDFLAGS) $(TESTDAEMON_LIBS)
$(LIB): $(LIB_OBJS)
@@ -49,6 +54,9 @@
$(SHAREDLIB): $(LIB_OBJS)
$(CXX) $(LDFLAGS) -shared $^ -o $@
+$(LIB_TEST): $(TESTLIB_OBJS) $(SHAREDLIB)
+ $(CXX) -o $@ $^ $(LDFLAGS) $(TESTLIB_LIBS)
+
%.o: %.cc
$(CXX) $(CXXFLAGS) -c $< -o $@
@@ -64,4 +72,5 @@
power_states.h
clean:
- rm -f $(CLIENT) $(DAEMON) $(LIB) $(SHAREDLIB) $(TESTDAEMON) *.o
+ rm -f $(CLIENT) $(DAEMON) $(LIB) $(SHAREDLIB) *.o
+ rm -f $(DAEMON_TEST) $(LIB_TEST)
diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index a66b570..6e01932 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -100,9 +100,6 @@
};
void MetricsDaemon::Run(bool run_as_daemon) {
- MetricsLibrary metrics_lib;
- metrics_lib.Init();
- Init(false, &metrics_lib);
if (!run_as_daemon || daemon(0, 0) == 0) {
Loop();
}
@@ -223,10 +220,10 @@
network_state_ == kNetworkStateOnline &&
power_state_ != kPowerStateMem) {
int online_time = static_cast<int>(now - network_state_last_);
- PublishMetric(kMetricTimeToNetworkDropName, online_time,
- kMetricTimeToNetworkDropMin,
- kMetricTimeToNetworkDropMax,
- kMetricTimeToNetworkDropBuckets);
+ SendMetric(kMetricTimeToNetworkDropName, online_time,
+ kMetricTimeToNetworkDropMin,
+ kMetricTimeToNetworkDropMax,
+ kMetricTimeToNetworkDropBuckets);
}
network_state_ = state;
@@ -349,10 +346,10 @@
// the usage to the nearest minute and sends it to UMA.
int minutes =
(record.seconds_ + kSecondsPerMinute / 2) / kSecondsPerMinute;
- PublishMetric(kMetricDailyUseTimeName, minutes,
- kMetricDailyUseTimeMin,
- kMetricDailyUseTimeMax,
- kMetricDailyUseTimeBuckets);
+ SendMetric(kMetricDailyUseTimeName, minutes,
+ kMetricDailyUseTimeMin,
+ kMetricDailyUseTimeMax,
+ kMetricDailyUseTimeBuckets);
// Truncates the usage file to ensure that no duplicate usage is
// sent to UMA.
@@ -445,8 +442,8 @@
usemon_interval_ = 0;
}
-void MetricsDaemon::PublishMetric(const char* name, int sample,
- int min, int max, int nbuckets) {
+void MetricsDaemon::SendMetric(const std::string& name, int sample,
+ int min, int max, int nbuckets) {
DLOG(INFO) << "received metric: " << name << " " << sample << " "
<< min << " " << max << " " << nbuckets;
metrics_lib_->SendToUMA(name, sample, min, max, nbuckets);
diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index 3aeaaaa..ba9e13c 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -30,6 +30,9 @@
usemon_source_(NULL) {}
~MetricsDaemon() {}
+ // Initializes.
+ void Init(bool testing, MetricsLibraryInterface* metrics_lib);
+
// Does all the work. If |run_as_daemon| is true, daemonizes by
// forking.
void Run(bool run_as_daemon);
@@ -48,8 +51,8 @@
FRIEND_TEST(MetricsDaemonTest, NetStateChangedSimpleDrop);
FRIEND_TEST(MetricsDaemonTest, NetStateChangedSuspend);
FRIEND_TEST(MetricsDaemonTest, PowerStateChanged);
- FRIEND_TEST(MetricsDaemonTest, PublishMetric);
FRIEND_TEST(MetricsDaemonTest, ScreenSaverStateChanged);
+ FRIEND_TEST(MetricsDaemonTest, SendMetric);
FRIEND_TEST(MetricsDaemonTest, SessionStateChanged);
FRIEND_TEST(MetricsDaemonTest, SetUserActiveStateSendOnLogin);
FRIEND_TEST(MetricsDaemonTest, SetUserActiveStateSendOnMonitor);
@@ -119,9 +122,6 @@
// Array of user session states.
static const char* kSessionStates_[kNumberSessionStates];
- // Initializes.
- void Init(bool testing, MetricsLibraryInterface* metrics_lib);
-
// Creates the event loop and enters it.
void Loop();
@@ -188,11 +188,11 @@
// Unschedules a scheduled use monitor, if any.
void UnscheduleUseMonitor();
- // Sends a stat to Chrome for transport to UMA (or prints it for
- // testing). See MetricsLibrary::SendToChrome in metrics_library.h
- // for a description of the arguments.
- void PublishMetric(const char* name, int sample,
- int min, int max, int nbuckets);
+ // Sends a regular (exponential) histogram sample to Chrome for
+ // transport to UMA. See MetricsLibrary::SendToUMA in
+ // metrics_library.h for a description of the arguments.
+ void SendMetric(const std::string& name, int sample,
+ int min, int max, int nbuckets);
// Test mode.
bool testing_;
diff --git a/metrics/metrics_daemon_main.cc b/metrics/metrics_daemon_main.cc
index 7ace333..f3f0714 100644
--- a/metrics/metrics_daemon_main.cc
+++ b/metrics/metrics_daemon_main.cc
@@ -10,7 +10,10 @@
DEFINE_bool(daemon, true, "run as daemon (use -nodaemon for debugging)");
int main(int argc, char** argv) {
- MetricsDaemon::MetricsDaemon d;
google::ParseCommandLineFlags(&argc, &argv, true);
- d.Run(FLAGS_daemon);
+ MetricsLibrary metrics_lib;
+ metrics_lib.Init();
+ MetricsDaemon daemon;
+ daemon.Init(false, &metrics_lib);
+ daemon.Run(FLAGS_daemon);
}
diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc
index 8180b14..2edbeda 100644
--- a/metrics/metrics_daemon_test.cc
+++ b/metrics/metrics_daemon_test.cc
@@ -25,7 +25,9 @@
class MetricsDaemonTest : public testing::Test {
protected:
virtual void SetUp() {
+ EXPECT_EQ(NULL, daemon_.daily_use_record_file_);
daemon_.Init(true, &metrics_lib_);
+ EXPECT_TRUE(NULL != daemon_.daily_use_record_file_);
daemon_.daily_use_record_file_ = kTestDailyUseRecordFile;
// The test fixture object will be used by the log message handler.
@@ -423,12 +425,6 @@
EXPECT_PRED_FORMAT2(AssertDailyUseRecord, /* day */ 7, /* seconds */ 30);
}
-TEST_F(MetricsDaemonTest, PublishMetric) {
- ExpectMetric("Dummy.Metric", 3, 1, 100, 50);
- daemon_.PublishMetric("Dummy.Metric", /* sample */ 3,
- /* min */ 1, /* max */ 100, /* buckets */ 50);
-}
-
TEST_F(MetricsDaemonTest, ScreenSaverStateChanged) {
EXPECT_EQ(MetricsDaemon::kUnknownScreenSaverState,
daemon_.screensaver_state_);
@@ -460,6 +456,12 @@
EXPECT_PRED_FORMAT2(AssertDailyUseRecord, /* day */ 5, /* seconds */ 200);
}
+TEST_F(MetricsDaemonTest, SendMetric) {
+ ExpectMetric("Dummy.Metric", 3, 1, 100, 50);
+ daemon_.SendMetric("Dummy.Metric", /* sample */ 3,
+ /* min */ 1, /* max */ 100, /* buckets */ 50);
+}
+
TEST_F(MetricsDaemonTest, SessionStateChanged) {
EXPECT_EQ(MetricsDaemon::kUnknownSessionState, daemon_.session_state_);
EXPECT_FALSE(daemon_.user_active_);
diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc
index 98415a8..6ec7226 100644
--- a/metrics/metrics_library.cc
+++ b/metrics/metrics_library.cc
@@ -16,16 +16,16 @@
static const char kAutotestPath[] =
"/var/log/metrics/autotest-events";
-static const char kChromePath[] =
+static const char kUMAEventsPath[] =
"/var/log/metrics/uma-events";
static const int32_t kBufferSize = 1024;
using namespace std;
// TODO(sosa@chromium.org) - use Chromium logger instead of stderr
-static void PrintError(const char *message, const char *file,
+static void PrintError(const char* message, const char* file,
int code) {
- const char *kProgramName = "metrics_library";
+ static const char *kProgramName = "libmetrics";
if (code == 0) {
fprintf(stderr, "%s: %s\n", kProgramName, message);
} else if (file == NULL) {
@@ -37,14 +37,16 @@
}
}
-// Sends message of size |length| to Chrome and returns true on success.
-static bool SendMessageToChrome(int32_t length, const char *message) {
- int chrome_fd = open(kChromePath,
+MetricsLibrary::MetricsLibrary()
+ : uma_events_file_(NULL) {}
+
+bool MetricsLibrary::SendMessageToChrome(int32_t length, const char* message) {
+ int chrome_fd = open(uma_events_file_,
O_WRONLY | O_APPEND | O_CREAT,
READ_WRITE_ALL_FILE_FLAGS);
// If we failed to open it, return.
if (chrome_fd < 0) {
- PrintError("open", kChromePath, errno);
+ PrintError("open", uma_events_file_, errno);
return false;
}
@@ -56,36 +58,28 @@
// Grab an exclusive lock to protect Chrome from truncating
// underneath us. Keep the file locked as briefly as possible.
if (flock(chrome_fd, LOCK_EX) < 0) {
- PrintError("flock", kChromePath, errno);
+ PrintError("flock", uma_events_file_, errno);
close(chrome_fd);
return false;
}
bool success = true;
if (write(chrome_fd, message, length) != length) {
- PrintError("write", kChromePath, errno);
+ PrintError("write", uma_events_file_, errno);
success = false;
}
// Release the file lock and close file.
if (flock(chrome_fd, LOCK_UN) < 0) {
- PrintError("unlock", kChromePath, errno);
+ PrintError("unlock", uma_events_file_, errno);
success = false;
}
close(chrome_fd);
return success;
}
-// Formats a name/value message for Chrome in |buffer| and returns the
-// length of the message or a negative value on error.
-//
-// Message format is: | LENGTH(binary) | NAME | \0 | VALUE | \0 |
-//
-// The arbitrary |format| argument covers the non-LENGTH portion of the
-// message. The caller is responsible to store the \0 character
-// between NAME and VALUE (e.g. "%s%c%d", name, '\0', value).
-static int32_t FormatChromeMessage(int32_t buffer_size, char *buffer,
- const char *format, ...) {
+int32_t MetricsLibrary::FormatChromeMessage(int32_t buffer_size, char* buffer,
+ const char* format, ...) {
int32_t message_length;
size_t len_size = sizeof(message_length);
@@ -115,9 +109,9 @@
}
void MetricsLibrary::Init() {
+ uma_events_file_ = kUMAEventsPath;
}
-// static
bool MetricsLibrary::SendToAutotest(const string& name, int value) {
FILE *autotest_file = fopen(kAutotestPath, "a+");
if (autotest_file == NULL) {
diff --git a/metrics/metrics_library.h b/metrics/metrics_library.h
index 2a6412c..8ae3f5b 100644
--- a/metrics/metrics_library.h
+++ b/metrics/metrics_library.h
@@ -5,9 +5,10 @@
#ifndef METRICS_LIBRARY_H_
#define METRICS_LIBRARY_H_
+#include <sys/types.h>
#include <string>
-// TODO(sosa@chromium.org): Add testing for send methods
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
class MetricsLibraryInterface {
public:
@@ -21,6 +22,8 @@
// Library used to send metrics to both Autotest and Chrome/UMA.
class MetricsLibrary : public MetricsLibraryInterface {
public:
+ MetricsLibrary();
+
// Initializes the library.
void Init();
@@ -51,6 +54,30 @@
// Sends to Autotest and returns true on success.
static bool SendToAutotest(const std::string& name, int value);
+
+ private:
+ friend class MetricsLibraryTest;
+ FRIEND_TEST(MetricsLibraryTest, FormatChromeMessage);
+ FRIEND_TEST(MetricsLibraryTest, FormatChromeMessageTooLong);
+ FRIEND_TEST(MetricsLibraryTest, SendMessageToChrome);
+ FRIEND_TEST(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation);
+
+ // Sends message of size |length| to Chrome for transport to UMA and
+ // returns true on success.
+ bool SendMessageToChrome(int32_t length, const char* message);
+
+ // Formats a name/value message for Chrome in |buffer| and returns the
+ // length of the message or a negative value on error.
+ //
+ // Message format is: | LENGTH(binary) | NAME | \0 | VALUE | \0 |
+ //
+ // The arbitrary |format| argument covers the non-LENGTH portion of the
+ // message. The caller is responsible to store the \0 character
+ // between NAME and VALUE (e.g. "%s%c%d", name, '\0', value).
+ int32_t FormatChromeMessage(int32_t buffer_size, char* buffer,
+ const char *format, ...);
+
+ const char* uma_events_file_;
};
#endif /* METRICS_LIBRARY_H_ */
diff --git a/metrics/metrics_library_test.cc b/metrics/metrics_library_test.cc
new file mode 100644
index 0000000..f585d35
--- /dev/null
+++ b/metrics/metrics_library_test.cc
@@ -0,0 +1,89 @@
+// Copyright (c) 2010 The Chromium OS 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 "metrics_library.h"
+
+#include <cstring>
+
+#include <base/file_util.h>
+#include <gtest/gtest.h>
+
+static const FilePath kTestUMAEventsFile("test-uma-events");
+
+class MetricsLibraryTest : public testing::Test {
+ protected:
+ virtual void SetUp() {
+ EXPECT_EQ(NULL, lib_.uma_events_file_);
+ lib_.Init();
+ EXPECT_TRUE(NULL != lib_.uma_events_file_);
+ lib_.uma_events_file_ = kTestUMAEventsFile.value().c_str();
+ }
+
+ virtual void TearDown() {
+ file_util::Delete(kTestUMAEventsFile, false);
+ }
+
+ MetricsLibrary lib_;
+};
+
+TEST_F(MetricsLibraryTest, FormatChromeMessage) {
+ char buf[7];
+ const int kLen = 6;
+ EXPECT_EQ(kLen, lib_.FormatChromeMessage(7, buf, "%d", 1));
+
+ char exp[kLen];
+ sprintf(exp, "%c%c%c%c1", kLen, 0, 0, 0);
+ EXPECT_EQ(0, memcmp(exp, buf, kLen));
+}
+
+TEST_F(MetricsLibraryTest, FormatChromeMessageTooLong) {
+ char buf[7];
+ EXPECT_EQ(-1, lib_.FormatChromeMessage(7, buf, "test"));
+}
+
+TEST_F(MetricsLibraryTest, SendEnumToUMA) {
+ char buf[100];
+ const int kLen = 40;
+ EXPECT_TRUE(lib_.SendEnumToUMA("Test.EnumMetric", 1, 3));
+ EXPECT_EQ(kLen, file_util::ReadFile(kTestUMAEventsFile, buf, 100));
+
+ char exp[kLen];
+ sprintf(exp, "%c%c%c%clinearhistogram%cTest.EnumMetric 1 3",
+ kLen, 0, 0, 0, 0);
+ EXPECT_EQ(0, memcmp(exp, buf, kLen));
+}
+
+TEST_F(MetricsLibraryTest, SendMessageToChrome) {
+ EXPECT_TRUE(lib_.SendMessageToChrome(4, "test"));
+ EXPECT_TRUE(lib_.SendMessageToChrome(7, "content"));
+ std::string uma_events;
+ EXPECT_TRUE(file_util::ReadFileToString(kTestUMAEventsFile, &uma_events));
+ EXPECT_EQ("testcontent", uma_events);
+}
+
+TEST_F(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation) {
+ // Checks that the library doesn't die badly if the file can't be
+ // created.
+ static const char kDoesNotExistFile[] = "/does/not/exist";
+ lib_.uma_events_file_ = kDoesNotExistFile;
+ static const char kDummyMessage[] = "Dummy Message";
+ EXPECT_FALSE(lib_.SendMessageToChrome(strlen(kDummyMessage), kDummyMessage));
+ file_util::Delete(FilePath(kDoesNotExistFile), false);
+}
+
+TEST_F(MetricsLibraryTest, SendToUMA) {
+ char buf[100];
+ const int kLen = 37;
+ EXPECT_TRUE(lib_.SendToUMA("Test.Metric", 2, 1, 100, 50));
+ EXPECT_EQ(kLen, file_util::ReadFile(kTestUMAEventsFile, buf, 100));
+
+ char exp[kLen];
+ sprintf(exp, "%c%c%c%chistogram%cTest.Metric 2 1 100 50", kLen, 0, 0, 0, 0);
+ EXPECT_EQ(0, memcmp(exp, buf, kLen));
+}
+
+int main(int argc, char** argv) {
+ testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+}
