Quantify instability according to the stability instrumentation

components/browser_watcher/postmortem_report_collector_unittest.cc

 // 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/browser_watcher/postmortem_report_collector.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/debug/activity_analyzer.h"
 #include "base/debug/activity_tracker.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/files/scoped_file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/persistent_memory_allocator.h"
 #include "base/process/process_handle.h"
 #include "base/stl_util.h"
+#include "base/test/histogram_tester.h"
 #include "base/threading/platform_thread.h"
 #include "components/browser_watcher/stability_data_names.h"
 #include "components/browser_watcher/stability_report_extractor.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/crashpad/crashpad/client/crash_report_database.h"
 
 namespace browser_watcher {
 
 using base::debug::ActivityData;
 using base::debug::ActivityTrackerMemoryAllocator;
 using base::debug::ActivityUserData;
 using base::debug::GlobalActivityTracker;
 using base::debug::ThreadActivityTracker;
 using base::File;
 using base::FilePersistentMemoryAllocator;
 using base::MemoryMappedFile;
 using base::PersistentMemoryAllocator;
 using base::WrapUnique;
 using crashpad::CrashReportDatabase;
 using crashpad::Settings;
 using crashpad::UUID;
 using testing::_;
+using testing::DoAll;
 using testing::Return;
 using testing::SetArgPointee;
 
 namespace {
 
 const char kProductName[] = "TestProduct";
 const char kVersionNumber[] = "TestVersionNumber";
 const char kChannelName[] = "TestChannel";
 
 // The tracker creates some data entries internally.
@@ skipping 98 matching lines @@
   message.SerializeToString(&expected_serialized);
   arg.SerializeToString(&actual_serialized);
   return expected_serialized == actual_serialized;
 }
 
 }  // namespace
 
 class PostmortemReportCollectorCollectAndSubmitAllPendingReportsTest
     : public testing::Test {
  public:
-  void SetUp() override {
-    testing::Test::SetUp();
+  void SetUpTest(bool system_session_clean) {
     // Create a dummy debug file.
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     debug_file_ = temp_dir_.GetPath().AppendASCII("foo-1.pma");
     {
       base::ScopedFILE file(base::OpenFile(debug_file_, "w"));
       ASSERT_NE(file.get(), nullptr);
     }
     ASSERT_TRUE(base::PathExists(debug_file_));
 
     // Expect collection of the debug file paths.
     debug_file_pattern_ = FILE_PATH_LITERAL("foo-*.pma");
     std::vector<base::FilePath> debug_files{debug_file_};
     EXPECT_CALL(collector_,
                 GetDebugStateFilePaths(debug_file_.DirName(),
                                        debug_file_pattern_, no_excluded_files_))
         .Times(1)
         .WillOnce(Return(debug_files));
 
     EXPECT_CALL(database_, GetSettings()).Times(1).WillOnce(Return(nullptr));
 
     // Expect a single collection call.
+    StabilityReport report;
+    report.mutable_system_state()->set_session_state(
+        system_session_clean ? SystemState::CLEAN : SystemState::UNCLEAN);
     EXPECT_CALL(collector_, CollectOneReport(debug_file_, _))
         .Times(1)
-        .WillOnce(Return(SUCCESS));
+        .WillOnce(DoAll(SetArgPointee<1>(report), Return(SUCCESS)));
 
     // Expect the call to write the proto to a minidump. This involves
     // requesting a report from the crashpad database, writing the report, then
     // finalizing it with the database.
     base::FilePath minidump_path = temp_dir_.GetPath().AppendASCII("foo-1.dmp");
     base::File minidump_file(
         minidump_path, base::File::FLAG_CREATE | base::File::File::FLAG_WRITE);
     crashpad::UUID new_report_uuid;
     new_report_uuid.InitializeWithNew();
     crashpad_report_ = {minidump_file.GetPlatformFile(), new_report_uuid,
                         minidump_path};
     EXPECT_CALL(database_, PrepareNewCrashReport(_))
         .Times(1)
         .WillOnce(DoAll(SetArgPointee<0>(&crashpad_report_),
                         Return(CrashReportDatabase::kNoError)));
 
     EXPECT_CALL(collector_,
                 WriteReportToMinidump(_, _, _, minidump_file.GetPlatformFile()))
         .Times(1)
         .WillOnce(Return(true));
   }
+  void ValidateHistograms(int unclean_cnt, int unclean_system_cnt) {
+    histogram_tester_.ExpectTotalCount(
+        "ActivityTracker.Collect.StabilityFileCount", 1);
+    histogram_tester_.ExpectBucketCount(
+        "ActivityTracker.Collect.StabilityFileCount", 1, 1);
+    histogram_tester_.ExpectTotalCount(
+        "ActivityTracker.Collect.UncleanShutdownCount", 1);
+    histogram_tester_.ExpectBucketCount(
+        "ActivityTracker.Collect.UncleanShutdownCount", unclean_cnt, 1);
+    histogram_tester_.ExpectTotalCount(
+        "ActivityTracker.Collect.UncleanSystemCount", 1);
+    histogram_tester_.ExpectBucketCount(
+        "ActivityTracker.Collect.UncleanSystemCount", unclean_system_cnt, 1);
+  }
+  void RunBasicTest(bool is_session_clean) {
+    SetUpTest(is_session_clean);
+
+    EXPECT_CALL(database_, FinishedWritingCrashReport(&crashpad_report_, _))
+        .Times(1)
+        .WillOnce(Return(CrashReportDatabase::kNoError));
+
+    // Run the test.
+    int success_cnt = collector_.CollectAndSubmitAllPendingReports(
+        debug_file_.DirName(), debug_file_pattern_, no_excluded_files_,
+        &database_);
+    ASSERT_EQ(1, success_cnt);
+    ASSERT_FALSE(base::PathExists(debug_file_));
+    ValidateHistograms(1, is_session_clean ? 0 : 1);
+  }
 
  protected:
+  base::HistogramTester histogram_tester_;
   base::ScopedTempDir temp_dir_;
   base::FilePath debug_file_;
   MockCrashReportDatabase database_;
   MockPostmortemReportCollector collector_;
   base::FilePath::StringType debug_file_pattern_;
   std::set<base::FilePath> no_excluded_files_;
   CrashReportDatabase::NewReport crashpad_report_;
 };
 
 TEST_F(PostmortemReportCollectorCollectAndSubmitAllPendingReportsTest,
-       CollectAndSubmitAllPendingReports) {
-  EXPECT_CALL(database_, FinishedWritingCrashReport(&crashpad_report_, _))
-      .Times(1)
-      .WillOnce(Return(CrashReportDatabase::kNoError));
+       CollectAndSubmitAllPendingReportsCleanSession) {
+  RunBasicTest(true);

[Sigurður Ásgeirsson, 2017/05/18 12:45:03]

ubernit: at the cost of a single additional line of code, you can make this so
much more readable.

Consider

RecordOneSession(<true|false>);
ValidateHistograms(...);

[manzagop (departed), 2017/05/18 14:52:12]

Done.

+}
 
-  // Run the test.
-  int success_cnt = collector_.CollectAndSubmitAllPendingReports(
-      debug_file_.DirName(), debug_file_pattern_, no_excluded_files_,
-      &database_);
-  ASSERT_EQ(1, success_cnt);
-  ASSERT_FALSE(base::PathExists(debug_file_));
+TEST_F(PostmortemReportCollectorCollectAndSubmitAllPendingReportsTest,
+       CollectAndSubmitAllPendingReportsUncleanSession) {
+  RunBasicTest(false);
 }
 
 TEST_F(PostmortemReportCollectorCollectAndSubmitAllPendingReportsTest,
        CollectAndSubmitAllPendingReportsStuckFile) {
+  SetUpTest(true);
+  base::HistogramTester histogram_tester;

[Sigurður Ásgeirsson, 2017/05/18 12:45:03]

do you need this in addition to the member variable?

[manzagop (departed), 2017/05/18 14:52:12]

Done.

+
   // Open the stability debug file to prevent its deletion.
   base::ScopedFILE file(base::OpenFile(debug_file_, "w"));
   ASSERT_NE(file.get(), nullptr);
 
   // Expect Crashpad is notified of an error writing the crash report.
   EXPECT_CALL(database_, ErrorWritingCrashReport(&crashpad_report_))
       .Times(1)
       .WillOnce(Return(CrashReportDatabase::kNoError));
 
   // Run the test.
   int success_cnt = collector_.CollectAndSubmitAllPendingReports(
       debug_file_.DirName(), debug_file_pattern_, no_excluded_files_,
       &database_);
   ASSERT_EQ(0, success_cnt);
   ASSERT_TRUE(base::PathExists(debug_file_));
+  ValidateHistograms(0, 0);
 }
 
 TEST(PostmortemReportCollectorTest, GetDebugStateFilePaths) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Create files.
   std::vector<base::FilePath> expected_paths;
   std::set<base::FilePath> excluded_paths;
   {
@@ skipping 486 matching lines @@
   PostmortemReportCollector collector(kProductName, kVersionNumber,
                                       kChannelName, &analyzer);
   StabilityReport report;
   ASSERT_EQ(SUCCESS, collector.CollectOneReport(debug_file_path(), &report));
 
   // Validate the report.
   ASSERT_EQ(SystemState::CLEAN, report.system_state().session_state());
 }
 
 }  // namespace browser_watcher