Normalizing source file names before calculating hash

chrome/browser/metrics/metrics_log_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 "chrome/browser/metrics/metrics_log.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/basictypes.h"
@@ skipping 456 matching lines @@
 
 // Test that we properly write profiler data to the log.
 TEST_F(MetricsLogTest, RecordProfilerData) {
   TestMetricsLog log(kClientId, kSessionId);
   EXPECT_EQ(0, log.uma_proto().profiler_event_size());
 
   {
     ProcessDataSnapshot process_data;
     process_data.process_id = 177;
     process_data.tasks.push_back(TaskSnapshot());
-    process_data.tasks.back().birth.location.file_name = "file";
+    process_data.tasks.back().birth.location.file_name = "a/b/file.h";
     process_data.tasks.back().birth.location.function_name = "function";
     process_data.tasks.back().birth.location.line_number = 1337;
     process_data.tasks.back().birth.thread_name = "birth_thread";
     process_data.tasks.back().death_data.count = 37;
     process_data.tasks.back().death_data.run_duration_sum = 31;
     process_data.tasks.back().death_data.run_duration_max = 17;
     process_data.tasks.back().death_data.run_duration_sample = 13;
     process_data.tasks.back().death_data.queue_duration_sum = 8;
     process_data.tasks.back().death_data.queue_duration_max = 5;
     process_data.tasks.back().death_data.queue_duration_sample = 3;
     process_data.tasks.back().death_thread_name = "Still_Alive";
     process_data.tasks.push_back(TaskSnapshot());
-    process_data.tasks.back().birth.location.file_name = "file2";
+    process_data.tasks.back().birth.location.file_name = "c\\d\\file2";
     process_data.tasks.back().birth.location.function_name = "function2";
     process_data.tasks.back().birth.location.line_number = 1773;
     process_data.tasks.back().birth.thread_name = "birth_thread2";
     process_data.tasks.back().death_data.count = 19;
     process_data.tasks.back().death_data.run_duration_sum = 23;
     process_data.tasks.back().death_data.run_duration_max = 11;
     process_data.tasks.back().death_data.run_duration_sample = 7;
     process_data.tasks.back().death_data.queue_duration_sum = 0;
     process_data.tasks.back().death_data.queue_duration_max = 0;
     process_data.tasks.back().death_data.queue_duration_sample = 0;
     process_data.tasks.back().death_thread_name = "death_thread";
 
     log.RecordProfilerData(process_data, content::PROCESS_TYPE_BROWSER);
     ASSERT_EQ(1, log.uma_proto().profiler_event_size());
     EXPECT_EQ(ProfilerEventProto::STARTUP_PROFILE,
               log.uma_proto().profiler_event(0).profile_type());
     EXPECT_EQ(ProfilerEventProto::WALL_CLOCK_TIME,
               log.uma_proto().profiler_event(0).time_source());
 
     ASSERT_EQ(2, log.uma_proto().profiler_event(0).tracked_object_size());
 
     const ProfilerEventProto::TrackedObject* tracked_object =
         &log.uma_proto().profiler_event(0).tracked_object(0);
-    EXPECT_EQ(GG_UINT64_C(10123486280357988687),
+    EXPECT_EQ(MetricsLogBase::Hash("file.h"),
               tracked_object->source_file_name_hash());
-    EXPECT_EQ(GG_UINT64_C(13962325592283560029),
+    EXPECT_EQ(MetricsLogBase::Hash("function"),
               tracked_object->source_function_name_hash());
     EXPECT_EQ(1337, tracked_object->source_line_number());
-    EXPECT_EQ(GG_UINT64_C(3400908935414830400),
+    EXPECT_EQ(MetricsLogBase::Hash("birth_thread"),
               tracked_object->birth_thread_name_hash());
     EXPECT_EQ(37, tracked_object->exec_count());
     EXPECT_EQ(31, tracked_object->exec_time_total());
     EXPECT_EQ(13, tracked_object->exec_time_sampled());
     EXPECT_EQ(8, tracked_object->queue_time_total());
     EXPECT_EQ(3, tracked_object->queue_time_sampled());
-    EXPECT_EQ(GG_UINT64_C(10151977472163283085),
+    EXPECT_EQ(MetricsLogBase::Hash("Still_Alive"),
               tracked_object->exec_thread_name_hash());
     EXPECT_EQ(177U, tracked_object->process_id());
     EXPECT_EQ(ProfilerEventProto::TrackedObject::BROWSER,
               tracked_object->process_type());
 
     tracked_object = &log.uma_proto().profiler_event(0).tracked_object(1);
-    EXPECT_EQ(GG_UINT64_C(2025659946535236365),
+    EXPECT_EQ(MetricsLogBase::Hash("file2"),
               tracked_object->source_file_name_hash());
-    EXPECT_EQ(GG_UINT64_C(55232426147951219),
+    EXPECT_EQ(MetricsLogBase::Hash("function2"),
               tracked_object->source_function_name_hash());
     EXPECT_EQ(1773, tracked_object->source_line_number());
-    EXPECT_EQ(GG_UINT64_C(1518842999910132863),
+    EXPECT_EQ(MetricsLogBase::Hash("birth_thread*"),
               tracked_object->birth_thread_name_hash());
     EXPECT_EQ(19, tracked_object->exec_count());
     EXPECT_EQ(23, tracked_object->exec_time_total());
     EXPECT_EQ(7, tracked_object->exec_time_sampled());
     EXPECT_EQ(0, tracked_object->queue_time_total());
     EXPECT_EQ(0, tracked_object->queue_time_sampled());
-    EXPECT_EQ(GG_UINT64_C(14275151213201158253),
+    EXPECT_EQ(MetricsLogBase::Hash("death_thread"),
               tracked_object->exec_thread_name_hash());
     EXPECT_EQ(177U, tracked_object->process_id());
     EXPECT_EQ(ProfilerEventProto::TrackedObject::BROWSER,
               tracked_object->process_type());
   }
 
   {
     ProcessDataSnapshot process_data;
     process_data.process_id = 1177;
     process_data.tasks.push_back(TaskSnapshot());
     process_data.tasks.back().birth.location.file_name = "file3";
     process_data.tasks.back().birth.location.function_name = "function3";
     process_data.tasks.back().birth.location.line_number = 7331;
     process_data.tasks.back().birth.thread_name = "birth_thread3";
     process_data.tasks.back().death_data.count = 137;
     process_data.tasks.back().death_data.run_duration_sum = 131;
     process_data.tasks.back().death_data.run_duration_max = 117;
     process_data.tasks.back().death_data.run_duration_sample = 113;
     process_data.tasks.back().death_data.queue_duration_sum = 108;
     process_data.tasks.back().death_data.queue_duration_max = 105;
     process_data.tasks.back().death_data.queue_duration_sample = 103;
     process_data.tasks.back().death_thread_name = "death_thread3";
+    process_data.tasks.push_back(TaskSnapshot());
+    process_data.tasks.back().birth.location.file_name = "";
+    process_data.tasks.back().birth.location.function_name = "";
+    process_data.tasks.back().birth.location.line_number = 7332;
+    process_data.tasks.back().birth.thread_name = "";
+    process_data.tasks.back().death_data.count = 138;
+    process_data.tasks.back().death_data.run_duration_sum = 132;
+    process_data.tasks.back().death_data.run_duration_max = 118;
+    process_data.tasks.back().death_data.run_duration_sample = 114;
+    process_data.tasks.back().death_data.queue_duration_sum = 109;
+    process_data.tasks.back().death_data.queue_duration_max = 106;
+    process_data.tasks.back().death_data.queue_duration_sample = 104;
+    process_data.tasks.back().death_thread_name = "";
 
     log.RecordProfilerData(process_data, content::PROCESS_TYPE_RENDERER);
     ASSERT_EQ(1, log.uma_proto().profiler_event_size());
     EXPECT_EQ(ProfilerEventProto::STARTUP_PROFILE,
               log.uma_proto().profiler_event(0).profile_type());
     EXPECT_EQ(ProfilerEventProto::WALL_CLOCK_TIME,
               log.uma_proto().profiler_event(0).time_source());
-    ASSERT_EQ(3, log.uma_proto().profiler_event(0).tracked_object_size());
+    ASSERT_EQ(4, log.uma_proto().profiler_event(0).tracked_object_size());
 
     const ProfilerEventProto::TrackedObject* tracked_object =
         &log.uma_proto().profiler_event(0).tracked_object(2);
-    EXPECT_EQ(GG_UINT64_C(2686523203278102732),
+    EXPECT_EQ(MetricsLogBase::Hash("file3"),

[Ilya Sherman, 2014/04/02 06:56:15]

IMO it's better to compare to a constant rather than to the output of a
function.  Otherwise, someone could come along and update MetricsLogBase::Hash()
without breaking this test... but that *would* likely break the server-side
processing.

[vadimt, 2014/04/02 17:50:25]

Replacing with metrics::HashMetricName (used in MetricsLogBase::Hash
implementation), which has a dedicated unit test.

[Ilya Sherman, 2014/04/02 22:55:16]

I would still prefer to keep a numeric constant as the expected value, rather
than the output of a function.  You can include both, if you really want to. 
The disadvantage of including *only* the function is that somebody can come
along and change the implementation of the function.  They'll know to fix the
tests.  They'll probably know to make sure the server is updated to decode
histogram names correctly.  I want to make sure that unittests remind them to
also update names used by the profiler data, which they might not even be aware
of.

[vadimt, 2014/04/03 00:41:48]

How about this?

               tracked_object->source_file_name_hash());
-    EXPECT_EQ(GG_UINT64_C(5081672290546182009),
+    EXPECT_EQ(MetricsLogBase::Hash("function3"),
               tracked_object->source_function_name_hash());
     EXPECT_EQ(7331, tracked_object->source_line_number());
-    EXPECT_EQ(GG_UINT64_C(1518842999910132863),
+    EXPECT_EQ(MetricsLogBase::Hash("birth_thread*"),
               tracked_object->birth_thread_name_hash());
     EXPECT_EQ(137, tracked_object->exec_count());
     EXPECT_EQ(131, tracked_object->exec_time_total());
     EXPECT_EQ(113, tracked_object->exec_time_sampled());
     EXPECT_EQ(108, tracked_object->queue_time_total());
     EXPECT_EQ(103, tracked_object->queue_time_sampled());
-    EXPECT_EQ(GG_UINT64_C(2203893603452504755),
+    EXPECT_EQ(MetricsLogBase::Hash("death_thread*"),
               tracked_object->exec_thread_name_hash());
     EXPECT_EQ(1177U, tracked_object->process_id());
     EXPECT_EQ(ProfilerEventProto::TrackedObject::RENDERER,
               tracked_object->process_type());
+
+    tracked_object = &log.uma_proto().profiler_event(0).tracked_object(3);
+    EXPECT_EQ(MetricsLogBase::Hash(""),
+              tracked_object->source_file_name_hash());
+    EXPECT_EQ(MetricsLogBase::Hash(""),
+              tracked_object->source_function_name_hash());
+    EXPECT_EQ(7332, tracked_object->source_line_number());
+    EXPECT_EQ(MetricsLogBase::Hash(""),
+              tracked_object->birth_thread_name_hash());
+    EXPECT_EQ(138, tracked_object->exec_count());
+    EXPECT_EQ(132, tracked_object->exec_time_total());
+    EXPECT_EQ(114, tracked_object->exec_time_sampled());
+    EXPECT_EQ(109, tracked_object->queue_time_total());
+    EXPECT_EQ(104, tracked_object->queue_time_sampled());
+    EXPECT_EQ(MetricsLogBase::Hash(""),
+              tracked_object->exec_thread_name_hash());
+    EXPECT_EQ(ProfilerEventProto::TrackedObject::RENDERER,
+              tracked_object->process_type());
   }
 }
 
 #if defined(OS_CHROMEOS)
 TEST_F(MetricsLogTest, MultiProfileUserCount) {
   std::string user1("user1@example.com");
   std::string user2("user2@example.com");
   std::string user3("user3@example.com");
 
   // |scoped_enabler| takes over the lifetime of |user_manager|.
@@ skipping 121 matching lines @@
   PairedDevice device2 =
       log.system_profile().hardware().bluetooth().paired_device(1);
 
   EXPECT_EQ(FakeBluetoothDeviceClient::kConfirmPasskeyClass,
             device2.bluetooth_class());
   EXPECT_EQ(PairedDevice::DEVICE_PHONE, device2.type());
   EXPECT_EQ(0x207D74U, device2.vendor_prefix());
   EXPECT_EQ(PairedDevice::VENDOR_ID_UNKNOWN, device2.vendor_id_source());
 }
 #endif  // OS_CHROMEOS