Add a mock class for DiskMountManagerObserver. disk_mount_manager_observer_unittests will be rewrit…

chromeos/disks/disk_mount_manager_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 <stddef.h>
 #include <stdint.h>
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/fake_cros_disks_client.h"
 #include "chromeos/disks/disk_mount_manager.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using base::MakeUnique;
+using base::StringPrintf;
 using chromeos::disks::DiskMountManager;
 using chromeos::CrosDisksClient;
 using chromeos::DBusThreadManager;
 using chromeos::FakeCrosDisksClient;
-using testing::_;
-using testing::Field;
-using testing::InSequence;
-using testing::InvokeWithoutArgs;
+using chromeos::MountType;
+using chromeos::disks::MountCondition;
 
 namespace {
 
 const char kReadOnlyMountpath[] = "/device/read_only_mount_path";
 const char kReadOnlyDeviceSource[] = "/device/read_only_source_path";
 
 // Holds information needed to create a DiskMountManager::Disk instance.
 struct TestDiskInfo {
   const char* source_path;
   const char* mount_path;
@@ skipping 110 matching lines @@
     chromeos::disks::MOUNT_CONDITION_NONE
   },
   {
     kReadOnlyDeviceSource,
     kReadOnlyMountpath,
     chromeos::MOUNT_TYPE_DEVICE,
     chromeos::disks::MOUNT_CONDITION_NONE
   },
 };
 
-// Mocks DiskMountManager observer.
+// Represents which function in |DiskMountManager::Observer| was invoked.
+enum ObserverEventType {
+  DEVICE_EVENT,  // OnDeviceEvent()
+  DISK_EVENT,    // OnDiskEvent()
+  FORMAT_EVENT,  // OnFormatEvent()
+  MOUNT_EVENT    // OnMountEvent()
+};
+
+// Represents every event notified to |DiskMountManager::Observer|.
+struct ObserverEvent {
+ public:
+  virtual ObserverEventType type() const = 0;
+};
+
+// Represents an invocation of |DiskMountManager::Observer::OnDeviceEvent()|.
+struct DeviceEvent : public ObserverEvent {
+  DiskMountManager::DeviceEvent event;
+  std::string device_path;
+
+  DeviceEvent() {}
+
+  DeviceEvent(DiskMountManager::DeviceEvent event,
+              const std::string& device_path)
+      : event(event), device_path(device_path) {}
+
+  ObserverEventType type() const override { return DEVICE_EVENT; }
+
+  bool operator==(const DeviceEvent& other) const {
+    return event == other.event && device_path == other.device_path;
+  }
+
+  std::string DebugString() const {
+    return StringPrintf("OnDeviceEvent(%d, %s)", event, device_path.c_str());
+  }
+};
+
+// Represents an invocation of |DiskMountManager::Observer::OnDiskEvent()|.
+struct DiskEvent : public ObserverEvent {
+  DiskMountManager::DiskEvent event;
+  const DiskMountManager::Disk* disk;
+
+  DiskEvent() {}
+
+  DiskEvent(DiskMountManager::DiskEvent event,
+            const DiskMountManager::Disk* disk)
+      : event(event), disk(disk) {}
+
+  DiskEvent(const DiskEvent& other) : event(other.event), disk(other.disk) {}
+
+  ObserverEventType type() const override { return DISK_EVENT; }
+
+  bool operator==(const DiskEvent& other) const {
+    return event == other.event && disk == other.disk;
+  }
+
+  std::string DebugString() const {
+    return StringPrintf("OnDiskEvent(event=%d, device_path=%s, mount_path=%s",
+                        event, disk->device_path().c_str(),
+                        disk->mount_path().c_str());
+  }
+};
+
+// OnFormatEvent()
+struct FormatEvent : public ObserverEvent {
+  DiskMountManager::FormatEvent event;
+  chromeos::FormatError error_code;
+  std::string device_path;
+
+  FormatEvent() {}
+  FormatEvent(DiskMountManager::FormatEvent event,
+              chromeos::FormatError error_code,
+              const std::string& device_path)
+      : event(event), error_code(error_code), device_path(device_path) {}
+
+  ObserverEventType type() const override { return FORMAT_EVENT; }
+
+  bool operator==(const FormatEvent& other) const {
+    return event == other.event && error_code == other.error_code &&
+           device_path == other.device_path;
+  }
+
+  std::string DebugString() const {
+    return StringPrintf("OnFormatEvent(%d, %d, %s)", event, error_code,
+                        device_path.c_str());
+  }
+};
+
+// Represents an invocation of |DiskMountManager::Observer::OnMountEvent()|.
+struct MountEvent : public ObserverEvent {
+  DiskMountManager::MountEvent event;
+  chromeos::MountError error_code;
+  DiskMountManager::MountPointInfo mount_point;
+
+  // Not passed to callback, but read by handlers. So it's captured upon
+  // callback.
+  std::shared_ptr<DiskMountManager::Disk> disk;
+
+  MountEvent()
+      : mount_point("",
+                    "",
+                    MountType::MOUNT_TYPE_INVALID,
+                    MountCondition::MOUNT_CONDITION_NONE) {}
+  ~MountEvent() {}
+
+  MountEvent(const MountEvent& other)
+      : event(other.event),
+        error_code(other.error_code),
+        mount_point(other.mount_point),
+        disk(other.disk) {}
+  MountEvent(DiskMountManager::MountEvent event,
+             chromeos::MountError error_code,
+             const DiskMountManager::MountPointInfo& mount_point,
+             std::shared_ptr<DiskMountManager::Disk> disk)
+      : event(event),
+        error_code(error_code),
+        mount_point(mount_point),
+        disk(disk) {}
+
+  ObserverEventType type() const override { return MOUNT_EVENT; }
+
+  bool operator==(const MountEvent& other) const;
+
+  std::string DebugString() const {
+    return StringPrintf("OnMountEvent(%d, %d, %s, %s, %d, %d)", event,
+                        error_code, mount_point.source_path.c_str(),
+                        mount_point.mount_path.c_str(), mount_point.mount_type,
+                        mount_point.mount_condition);
+  }
+};
+
+// A mock |Observer| class which records all invocation of the methods invoked
+// from DiskMountManager and all the arguments passed to them.
 class MockDiskMountManagerObserver : public DiskMountManager::Observer {
  public:
-  virtual ~MockDiskMountManagerObserver() {}
-
-  MOCK_METHOD2(OnDiskEvent, void(DiskMountManager::DiskEvent event,
-                                 const DiskMountManager::Disk* disk));
-  MOCK_METHOD2(OnDeviceEvent, void(DiskMountManager::DeviceEvent event,
-                                   const std::string& device_path));
-  MOCK_METHOD3(OnMountEvent,
-      void(DiskMountManager::MountEvent event,
-           chromeos::MountError error_code,
-           const DiskMountManager::MountPointInfo& mount_point));
-  MOCK_METHOD3(OnFormatEvent,
-      void(DiskMountManager::FormatEvent event,
-           chromeos::FormatError error_code,
-           const std::string& device_path));
-};
-
-// Expect |is_read_only| value of a disk object keyed by |source_path|.
-void ExpectDiskReadOnly(const DiskMountManager* manager,
-                        const std::string& source_path,
-                        bool expected) {
-  EXPECT_EQ(expected,
-            manager->disks().find(source_path)->second->is_read_only());
+  MockDiskMountManagerObserver(const DiskMountManager* manager)
+      : manager_(manager) {}
+  ~MockDiskMountManagerObserver() override {}
+
+  // Mock notify methods.
+  void OnDeviceEvent(DiskMountManager::DeviceEvent event,
+                     const std::string& device_path) override {
+    events_.push_back(MakeUnique<DeviceEvent>(event, device_path));
+  }
+
+  void OnDiskEvent(DiskMountManager::DiskEvent event,
+                   const DiskMountManager::Disk* disk) override {
+    // Take a snapshot (copy) of the Disk object at the time of invocation for
+    // later verification.
+    events_.push_back(
+        MakeUnique<DiskEvent>(event, new DiskMountManager::Disk(*disk)));
+  }
+
+  void OnFormatEvent(DiskMountManager::FormatEvent event,
+                     chromeos::FormatError error_code,
+                     const std::string& device_path) override {
+    events_.push_back(MakeUnique<FormatEvent>(event, error_code, device_path));
+  }
+
+  void OnMountEvent(
+      DiskMountManager::MountEvent event,
+      chromeos::MountError error_code,
+      const DiskMountManager::MountPointInfo& mount_point) override {
+    // Take a snapshot (copy) of a Disk object at the time of invocation.
+    // It can be verified later besides the arguments.
+    events_.push_back(MakeUnique<MountEvent>(
+        event, error_code, mount_point,
+        std::shared_ptr<DiskMountManager::Disk>(new DiskMountManager::Disk(
+            *manager_->disks().find(mount_point.source_path)->second))));
+  }
+
+  // Get invocation history to be verified by testcases.
+  // Verify if the |index|th invocation is OnDeviceEvent() and return details.

[satorux1, 2016/09/06 07:08:46]

Nit: Get->Gets Verify->Verifies

https://google.github.io/styleguide/cppguide.html#Function_Comments

Please fix other places too

[yamaguchi, 2016/09/06 09:29:03]

Done.

+  DeviceEvent GetDeviceEvent(size_t index) {
+    DCHECK_GT(events_.size(), index);
+    DCHECK_EQ(DEVICE_EVENT, events_[index]->type());
+    return static_cast<const DeviceEvent&>(*events_[index]);
+  }
+
+  // Verify if the |index|th invocation is OnDiskEvent() and return details.
+  DiskEvent GetDiskEvent(size_t index) {
+    DCHECK_GT(events_.size(), index);
+    DCHECK_EQ(DISK_EVENT, events_[index]->type());
+    return static_cast<const DiskEvent&>(*events_[index]);
+  }
+
+  // Verify if the |index|th invocation is OnFormatEvent() and return details.
+  FormatEvent GetFormatEvent(size_t index) {
+    DCHECK_GT(events_.size(), index);
+    DCHECK_EQ(FORMAT_EVENT, events_[index]->type());
+    return static_cast<const FormatEvent&>(*events_[index]);
+  }
+
+  // Verify if the |index|th invocation is OnMountEvent() and return details.
+  MountEvent GetMountEvent(size_t index) {
+    DCHECK_GT(events_.size(), index);
+    DCHECK_EQ(MOUNT_EVENT, events_[index]->type());
+    return static_cast<const MountEvent&>(*events_[index]);
+  }
+
+  // Returns number of callback invocations happened so far.
+  size_t GetEventCount() { return events_.size(); }
+
+  // Counts the number of |MountEvent| recorded so far that matches the given
+  // condition.
+  size_t CountMountEvents(DiskMountManager::MountEvent mount_event_type,
+                          chromeos::MountError error_code,
+                          const std::string& mount_path) {

[satorux1, 2016/09/06 07:08:46]

Cannot this take const MountEvent& just like CountFormatEvents() below? If
that's possible, maybe we can generalize the two functions into one function
CountEvents() that takes const ObserverEvent& ?

[yamaguchi, 2016/09/06 09:29:03]

I think taking (unnamed namespace)::MountEvent& in this case will make the test
a slightly misleading as it'd require to build the entire expected object.
This function only compares some limited fields in the object which the test
needs to, like the existing code does by testing::Field.
I think we may feed predicates to generalized CountEvent() and use
std::count_if, but I think this would be acceptable for now as there are only 2
patterns.

[satorux1, 2016/09/08 06:24:21]

Ah OK, but MountEvent has only 4 members and 3 of them are tested here. If the
other one (disk) can be optional, taking the entire object may be ok? It's up to
you.

[yamaguchi, 2016/09/08 09:05:47]

One concern was that Disk doesn't have a constructor with no arguments, but a
one with all 20 field values. Either the test cases or another constructor
MountEvent would need to contain a call to that constructor and I thought that
it may make the test somewhat redundant, compared to the existing code. So I
prefer this way for now.

+    size_t matched = 0;
+    for (auto& it : events_) {
+      if (it->type() != MOUNT_EVENT) {
+        continue;
+      }

[satorux1, 2016/09/06 07:08:46]

nit: {} is generally omitted for simple single-line statements like this. The
style guide says:

[yamaguchi, 2016/09/06 09:29:03]

Done.

+      const MountEvent& mount_event = static_cast<const MountEvent&>(*it);
+      if (mount_event.event == mount_event_type &&
+          mount_event.error_code == error_code &&
+          mount_event.mount_point.mount_path == mount_path) {
+        matched++;
+      }
+    }
+    return matched;
+  }
+
+  // Counts the number of |FormatEvent| recorded so far that matches with
+  // |format_event|.
+  size_t CountFormatEvents(const FormatEvent& exptected_format_event) {
+    size_t matched = 0;
+    for (auto& it : events_) {
+      if (it->type() != FORMAT_EVENT) {
+        continue;
+      }
+      if (static_cast<const FormatEvent&>(*it) == exptected_format_event) {
+        matched++;
+      }
+    }
+    return matched;
+  }
+
+ private:
+  // Pointer to the manager object to which this |Observer| is registered.
+  const DiskMountManager* manager_;
+
+  // Records all invocations.
+  std::vector<std::unique_ptr<ObserverEvent>> events_;
+};
+
+// Shift operators of ostream.
+// Needed to print values in case of EXPECT_* failure in gtest.
+std::ostream& operator<<(std::ostream& stream,
+                         const DeviceEvent& device_event) {
+  return stream << device_event.DebugString();
+}
+
+std::ostream& operator<<(std::ostream& stream, const DiskEvent& disk_event) {
+  return stream << disk_event.DebugString();
+}
+
+std::ostream& operator<<(std::ostream& stream,
+                         const FormatEvent& format_event) {
+  return stream << format_event.DebugString();
+}
+
+std::ostream& operator<<(std::ostream& stream, const MountEvent& mount_event) {
+  return stream << mount_event.DebugString();
 }
 
 class DiskMountManagerTest : public testing::Test {
  public:
-  DiskMountManagerTest() {}
+  DiskMountManagerTest() : observer_(NULL) {}
   ~DiskMountManagerTest() override {}
 
   // Sets up test dbus tread manager and disks mount manager.
   // Initializes disk mount manager disks and mount points.
   // Adds a test observer to the disk mount manager.
   void SetUp() override {
     fake_cros_disks_client_ = new FakeCrosDisksClient;
     DBusThreadManager::GetSetterForTesting()->SetCrosDisksClient(
         std::unique_ptr<CrosDisksClient>(fake_cros_disks_client_));
 
     DiskMountManager::Initialize();
 
     InitDisksAndMountPoints();
 
-    DiskMountManager::GetInstance()->AddObserver(&observer_);
+    observer_ =
+        new MockDiskMountManagerObserver(DiskMountManager::GetInstance());
+    DiskMountManager::GetInstance()->AddObserver(observer_);
   }
 
   // Shuts down dbus thread manager and disk moutn manager used in the test.
   void TearDown() override {
-    DiskMountManager::GetInstance()->RemoveObserver(&observer_);
+    DiskMountManager::GetInstance()->RemoveObserver(observer_);
     DiskMountManager::Shutdown();
     DBusThreadManager::Shutdown();
   }
 
  protected:
   // Checks if disk mount manager contains a mount point with specified moutn
   // path.
   bool HasMountPoint(const std::string& mount_path) {
     const DiskMountManager::MountPointMap& mount_points =
         DiskMountManager::GetInstance()->mount_points();
@@ skipping 43 matching lines @@
     // expected that the corresponding disk is already added).
     for (size_t i = 0; i < arraysize(kTestDisks); i++)
       AddTestDisk(kTestDisks[i]);
 
     for (size_t i = 0; i < arraysize(kTestMountPoints); i++)
       AddTestMountPoint(kTestMountPoints[i]);
   }
 
  protected:
   chromeos::FakeCrosDisksClient* fake_cros_disks_client_;
-  MockDiskMountManagerObserver observer_;
+  MockDiskMountManagerObserver* observer_;
   base::MessageLoopForUI message_loop_;
 };
 
 // Tests that the observer gets notified on attempt to format non existent mount
 // point.
 TEST_F(DiskMountManagerTest, Format_NotMounted) {
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                       chromeos::FORMAT_ERROR_UNKNOWN,
-                                       "/mount/non_existent"))
-      .Times(1);
   DiskMountManager::GetInstance()->FormatMountedDevice("/mount/non_existent");
+  ASSERT_GE(1U, observer_->GetEventCount());
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/mount/non_existent"),
+            observer_->GetFormatEvent(0));
 }
 
 // Tests that the observer gets notified on attempt to format read-only mount
 // point.
 TEST_F(DiskMountManagerTest, Format_ReadOnly) {
-  EXPECT_CALL(observer_,
-              OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                            chromeos::FORMAT_ERROR_DEVICE_NOT_ALLOWED,
-                            kReadOnlyMountpath))
-      .Times(1);
   DiskMountManager::GetInstance()->FormatMountedDevice(kReadOnlyMountpath);
+  ASSERT_GE(1U, observer_->GetEventCount());
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_DEVICE_NOT_ALLOWED,
+                        kReadOnlyMountpath),
+            observer_->GetFormatEvent(0));
 }
 
 // Tests that it is not possible to format archive mount point.
 TEST_F(DiskMountManagerTest, Format_Archive) {
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                       chromeos::FORMAT_ERROR_UNKNOWN,
-                                       "/archive/source_path"))
-      .Times(1);
-
   DiskMountManager::GetInstance()->FormatMountedDevice("/archive/mount_path");
+  ASSERT_GE(1U, observer_->GetEventCount());
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/archive/source_path"),
+            observer_->GetFormatEvent(0));
 }
 
 // Tests that format fails if the device cannot be unmounted.
 TEST_F(DiskMountManagerTest, Format_FailToUnmount) {
   // Before formatting mounted device, the device should be unmounted.
   // In this test unmount will fail, and there should be no attempt to
   // format the device.
 
-  // Set up expectations for observer mock.
-  // Observer should be notified that unmount attempt fails and format task
-  // failed to start.
-  {
-    InSequence s;
-
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_INTERNAL,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_UNKNOWN,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
   fake_cros_disks_client_->MakeUnmountFail();
   // Start test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
   // Cros disks will respond asynchronoulsy, so let's drain the message loop.
   message_loop_.RunUntilIdle();
 
+  // Observer should be notified that unmount attempt fails and format task
+  // failed to start.
+  ASSERT_GE(2U, observer_->GetEventCount());
+  MountEvent mount_event = observer_->GetMountEvent(0);
+  EXPECT_EQ(DiskMountManager::UNMOUNTING, mount_event.event);
+  EXPECT_EQ(chromeos::MOUNT_ERROR_INTERNAL, mount_event.error_code);
+  EXPECT_EQ("/device/mount_path", mount_event.mount_point.mount_path);
+
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path"),
+            observer_->GetFormatEvent(1));
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(0, fake_cros_disks_client_->format_call_count());
 
   // The device mount should still be here.
   EXPECT_TRUE(HasMountPoint("/device/mount_path"));
 }
 
 // Tests that observer is notified when cros disks fails to start format
 // process.
 TEST_F(DiskMountManagerTest, Format_FormatFailsToStart) {
   // Before formatting mounted device, the device should be unmounted.
   // In this test, unmount will succeed, but call to Format method will
   // fail.
 
-  // Set up expectations for observer mock.
-  // Observer should be notified that the device was unmounted and format task
-  // failed to start.
-  {
-    InSequence s;
-
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_NONE,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_UNKNOWN,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
   fake_cros_disks_client_->MakeFormatFail();
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
   // Cros disks will respond asynchronoulsy, so let's drain the message loop.
   message_loop_.RunUntilIdle();
 
+  // Observer should be notified that the device was unmounted and format task
+  // failed to start.
+  ASSERT_GE(2U, observer_->GetEventCount());
+  MountEvent mount_event = observer_->GetMountEvent(0);
+  EXPECT_EQ(DiskMountManager::UNMOUNTING, mount_event.event);
+  EXPECT_EQ(chromeos::MOUNT_ERROR_NONE, mount_event.error_code);
+  EXPECT_EQ("/device/mount_path", mount_event.mount_point.mount_path);
+
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path"),
+            observer_->GetFormatEvent(1));
+
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
             fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device mount should be gone.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
 }
 
 // Tests the case where there are two format requests for the same device.
 TEST_F(DiskMountManagerTest, Format_ConcurrentFormatCalls) {
   // Only the first format request should be processed (the second unmount
   // request fails because the device is already unmounted at that point).
   // CrosDisksClient will report that the format process for the first request
   // is successfully started.
 
-  // Set up expectations for observer mock.
+  fake_cros_disks_client_->set_unmount_listener(
+      base::Bind(&FakeCrosDisksClient::MakeUnmountFail,
+                 base::Unretained(fake_cros_disks_client_)));
+  // Start the test.
+  DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
+  DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
+
+  // Cros disks will respond asynchronoulsy, so let's drain the message loop.
+  message_loop_.RunUntilIdle();
+
   // The observer should get a FORMAT_STARTED event for one format request and a
   // FORMAT_COMPLETED with an error code for the other format request. The
   // formatting will be started only for the first request.
   // There should be only one UNMOUNTING event. The result of the second one
   // should not be reported as the mount point will go away after the first
   // request.
   //
   // Note that in this test the format completion signal will not be simulated,
   // so the observer should not get FORMAT_COMPLETED signal.
-  {
-    InSequence s;
 
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_NONE,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_UNKNOWN,
-                                         "/device/source_path"))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
-                                         chromeos::FORMAT_ERROR_NONE,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
-  fake_cros_disks_client_->set_unmount_listener(
-      base::Bind(&FakeCrosDisksClient::MakeUnmountFail,
-                 base::Unretained(fake_cros_disks_client_)));
-  // Start the test.
-  DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
-  DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
-
-  // Cros disks will respond asynchronoulsy, so let's drain the message loop.
-  message_loop_.RunUntilIdle();
+  ASSERT_GE(3U, observer_->GetEventCount());
+  MountEvent mount_event = observer_->GetMountEvent(0);
+  EXPECT_EQ(DiskMountManager::UNMOUNTING, mount_event.event);
+  EXPECT_EQ(chromeos::MOUNT_ERROR_NONE, mount_event.error_code);
+  EXPECT_EQ("/device/mount_path", mount_event.mount_point.mount_path);
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path"),
+            observer_->GetFormatEvent(1));
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_STARTED,
+                        chromeos::FORMAT_ERROR_NONE, "/device/source_path"),
+            observer_->GetFormatEvent(2));
 
   EXPECT_EQ(2, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
             fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat",
             fake_cros_disks_client_->last_format_filesystem());
 
   // The device mount should be gone.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
 }
 
+// Verifies a |MountEvent| with the given condition. This function only checks
+// the |mount_path| in |MountPointInfo| to make sure to match the event with
+// preceding mount invocations.
+void VerifyMountEvent(MountEvent mount_event,
+                      DiskMountManager::MountEvent mount_event_type,
+                      chromeos::MountError error_code,
+                      const std::string& mount_path) {
+  EXPECT_EQ(mount_event_type, mount_event.event);
+  EXPECT_EQ(error_code, mount_event.error_code);
+  EXPECT_EQ(mount_path, mount_event.mount_point.mount_path);
+}
+
 // Tests the case when the format process actually starts and fails.
 TEST_F(DiskMountManagerTest, Format_FormatFails) {
   // Both unmount and format device cals are successful in this test.
 
-  // Set up expectations for observer mock.
-  // The observer should get notified that the device was unmounted and that
-  // formatting has started.
-  // After the formatting starts, the test will simulate failing
-  // FORMAT_COMPLETED signal, so the observer should also be notified the
-  // formatting has failed (FORMAT_COMPLETED event).
-  {
-    InSequence s;
-
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_NONE,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
-                                         chromeos::FORMAT_ERROR_NONE,
-                                         "/device/source_path"))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_UNKNOWN,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
   // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
             fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device should be unmounted by now.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
 
   // Send failing FORMAT_COMPLETED signal.
   // The failure is marked by ! in fromt of the path (but this should change
   // soon).
   fake_cros_disks_client_->SendFormatCompletedEvent(
       chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path");
+
+  // The observer should get notified that the device was unmounted and that
+  // formatting has started.
+  // After the formatting starts, the test will simulate failing
+  // FORMAT_COMPLETED signal, so the observer should also be notified the
+  // formatting has failed (FORMAT_COMPLETED event).
+  ASSERT_EQ(3U, observer_->GetEventCount());
+  VerifyMountEvent(observer_->GetMountEvent(0), DiskMountManager::UNMOUNTING,
+                   chromeos::MOUNT_ERROR_NONE, "/device/mount_path");
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_STARTED,
+                        chromeos::FORMAT_ERROR_NONE, "/device/source_path"),
+            observer_->GetFormatEvent(1));
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_UNKNOWN, "/device/source_path"),
+            observer_->GetFormatEvent(2));
 }
 
 // Tests the case when formatting completes successfully.
 TEST_F(DiskMountManagerTest, Format_FormatSuccess) {
   // Set up cros disks client mocks.
   // Both unmount and format device cals are successful in this test.
 
-  // Set up expectations for observer mock.
-  // The observer should receive UNMOUNTING, FORMAT_STARTED and FORMAT_COMPLETED
-  // events (all of them without an error set).
-  {
-    InSequence s;
-
-    EXPECT_CALL(observer_,
-        OnMountEvent(DiskMountManager::UNMOUNTING,
-                     chromeos::MOUNT_ERROR_NONE,
-                     Field(&DiskMountManager::MountPointInfo::mount_path,
-                           "/device/mount_path")))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
-                                         chromeos::FORMAT_ERROR_NONE,
-                                         "/device/source_path"))
-        .Times(1);
-
-    EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                         chromeos::FORMAT_ERROR_NONE,
-                                         "/device/source_path"))
-        .Times(1);
-  }
-
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
   // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(1, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
             fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // The device should be unmounted by now.
   EXPECT_FALSE(HasMountPoint("/device/mount_path"));
 
   // Simulate cros_disks reporting success.
   fake_cros_disks_client_->SendFormatCompletedEvent(
       chromeos::FORMAT_ERROR_NONE, "/device/source_path");
+
+  // The observer should receive UNMOUNTING, FORMAT_STARTED and FORMAT_COMPLETED
+  // events (all of them without an error set).
+  ASSERT_GE(3U, observer_->GetEventCount());
+  VerifyMountEvent(observer_->GetMountEvent(0), DiskMountManager::UNMOUNTING,
+                   chromeos::MOUNT_ERROR_NONE, "/device/mount_path");
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_STARTED,
+                        chromeos::FORMAT_ERROR_NONE, "/device/source_path"),
+            observer_->GetFormatEvent(1));
+  EXPECT_EQ(FormatEvent(DiskMountManager::FORMAT_COMPLETED,
+                        chromeos::FORMAT_ERROR_NONE, "/device/source_path"),
+            observer_->GetFormatEvent(2));
 }
 
 // Tests that it's possible to format the device twice in a row (this may not be
 // true if the list of pending formats is not properly cleared).
 TEST_F(DiskMountManagerTest, Format_ConsecutiveFormatCalls) {
   // All unmount and format device cals are successful in this test.
   // Each of the should be made twice (once for each formatting task).
 
-  // Set up expectations for observer mock.
-  // The observer should receive UNMOUNTING, FORMAT_STARTED and FORMAT_COMPLETED
-  // events (all of them without an error set) twice (once for each formatting
-  // task).
-  // Also, there should be a MOUNTING event when the device remounting is
-  // simulated.
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_COMPLETED,
-                                       chromeos::FORMAT_ERROR_NONE,
-                                       "/device/source_path"))
-      .Times(2);
-
-  EXPECT_CALL(observer_, OnFormatEvent(DiskMountManager::FORMAT_STARTED,
-                                       chromeos::FORMAT_ERROR_NONE,
-                                       "/device/source_path"))
-      .Times(2);
-
-  EXPECT_CALL(observer_,
-      OnMountEvent(DiskMountManager::UNMOUNTING,
-                   chromeos::MOUNT_ERROR_NONE,
-                   Field(&DiskMountManager::MountPointInfo::mount_path,
-                         "/device/mount_path")))
-      .Times(2);
-
-  EXPECT_CALL(observer_,
-      OnMountEvent(DiskMountManager::MOUNTING,
-                   chromeos::MOUNT_ERROR_NONE,
-                   Field(&DiskMountManager::MountPointInfo::mount_path,
-                         "/device/mount_path")))
-      .Times(1);
-
   // Start the test.
   DiskMountManager::GetInstance()->FormatMountedDevice("/device/mount_path");
 
   // Wait for Unmount and Format calls to end.
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1, fake_cros_disks_client_->unmount_call_count());
   EXPECT_EQ("/device/mount_path",
             fake_cros_disks_client_->last_unmount_device_path());
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
@@ skipping 31 matching lines @@
   EXPECT_EQ(chromeos::UNMOUNT_OPTIONS_NONE,
             fake_cros_disks_client_->last_unmount_options());
   EXPECT_EQ(2, fake_cros_disks_client_->format_call_count());
   EXPECT_EQ("/device/source_path",
             fake_cros_disks_client_->last_format_device_path());
   EXPECT_EQ("vfat", fake_cros_disks_client_->last_format_filesystem());
 
   // Simulate cros_disks reporting success.
   fake_cros_disks_client_->SendFormatCompletedEvent(
       chromeos::FORMAT_ERROR_NONE, "/device/source_path");
+
+  // The observer should receive UNMOUNTING, FORMAT_STARTED and FORMAT_COMPLETED
+  // events (all of them without an error set) twice (once for each formatting
+  // task).
+  // Also, there should be a MOUNTING event when the device remounting is
+  // simulated.
+  EXPECT_EQ(7U, observer_->GetEventCount());
+
+  EXPECT_EQ(2U, observer_->CountFormatEvents(FormatEvent(
+                    DiskMountManager::FORMAT_COMPLETED,
+                    chromeos::FORMAT_ERROR_NONE, "/device/source_path")));
+
+  EXPECT_EQ(2U, observer_->CountFormatEvents(FormatEvent(
+                    DiskMountManager::FORMAT_STARTED,
+                    chromeos::FORMAT_ERROR_NONE, "/device/source_path")));
+
+  EXPECT_EQ(2U, observer_->CountMountEvents(DiskMountManager::UNMOUNTING,
+                                            chromeos::MOUNT_ERROR_NONE,
+                                            "/device/mount_path"));
+
+  EXPECT_EQ(1U, observer_->CountMountEvents(DiskMountManager::MOUNTING,
+                                            chromeos::MOUNT_ERROR_NONE,
+                                            "/device/mount_path"));
 }
 
 TEST_F(DiskMountManagerTest, MountPath_RecordAccessMode) {
   DiskMountManager* manager = DiskMountManager::GetInstance();
   const std::string kSourcePath1 = "/device/source_path";
   const std::string kSourcePath2 = "/device/source_path2";
   const std::string kSourceFormat = std::string();
   const std::string kMountLabel = std::string();  // N/A for MOUNT_TYPE_DEVICE
   // For MountCompleted. Must be non-empty strings.
   const std::string kMountPath1 = "/media/foo";
   const std::string kMountPath2 = "/media/bar";
 
-  // Event handlers of observers should be called.
-  EXPECT_CALL(
-      observer_,
-      OnMountEvent(
-          DiskMountManager::MOUNTING, chromeos::MOUNT_ERROR_NONE,
-          Field(&DiskMountManager::MountPointInfo::mount_path, kMountPath1)));
-  // For the 2nd source, the disk (block device) is not read-only but the
-  // test will mount it in read-only mode.
-  // Observers query |disks_| from |DiskMountManager| in its event handler for
-  // a mount completion event. Therefore |disks_| must be updated with correct
-  // |read_only| value before notifying to observers.
-  EXPECT_CALL(
-      observer_,
-      OnMountEvent(
-          DiskMountManager::MOUNTING, chromeos::MOUNT_ERROR_NONE,
-          Field(&DiskMountManager::MountPointInfo::mount_path, kMountPath2)))
-      .WillOnce(InvokeWithoutArgs(
-          // Verify if the disk appears read-only at the time of notification
-          // to observers.
-          [&]() { ExpectDiskReadOnly(manager, kSourcePath2, true); }));
-
   manager->MountPath(kSourcePath1, kSourceFormat, std::string(),
                      chromeos::MOUNT_TYPE_DEVICE,
                      chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
   manager->MountPath(kSourcePath2, kSourceFormat, std::string(),
                      chromeos::MOUNT_TYPE_DEVICE,
                      chromeos::MOUNT_ACCESS_MODE_READ_ONLY);
   // Simulate cros_disks reporting mount completed.
   fake_cros_disks_client_->SendMountCompletedEvent(
       chromeos::MOUNT_ERROR_NONE, kSourcePath1, chromeos::MOUNT_TYPE_DEVICE,
       kMountPath1);
   fake_cros_disks_client_->SendMountCompletedEvent(
       chromeos::MOUNT_ERROR_NONE, kSourcePath2, chromeos::MOUNT_TYPE_DEVICE,
       kMountPath2);
 
+  // Event handlers of observers should be called.
+  ASSERT_GE(2U, observer_->GetEventCount());
+  VerifyMountEvent(observer_->GetMountEvent(0), DiskMountManager::MOUNTING,
+                   chromeos::MOUNT_ERROR_NONE, kMountPath1);
+  // For the 2nd source, the disk (block device) is not read-only but the
+  // test will mount it in read-only mode.
+  // Observers query |disks_| from |DiskMountManager| in its event handler for
+  // a mount completion event. Therefore |disks_| must be updated with correct
+  // |read_only| value before notifying to observers.
+  MountEvent secondMountEvent = observer_->GetMountEvent(1);
+  EXPECT_EQ(DiskMountManager::MOUNTING, secondMountEvent.event);
+  EXPECT_EQ(chromeos::MOUNT_ERROR_NONE, secondMountEvent.error_code);
+  EXPECT_EQ(kMountPath2, secondMountEvent.mount_point.mount_path);
+  // Verify if the disk appears read-only at the time of notification to
+  // observers.
+  EXPECT_TRUE(secondMountEvent.disk->is_read_only());
+
+  // Verify the final state of manager->disks.
   const DiskMountManager::DiskMap& disks = manager->disks();
   ASSERT_GT(disks.count(kSourcePath1), 0U);
   EXPECT_FALSE(disks.find(kSourcePath1)->second->is_read_only());
   ASSERT_GT(disks.count(kSourcePath2), 0U);
   EXPECT_TRUE(disks.find(kSourcePath2)->second->is_read_only());
 }
 
 TEST_F(DiskMountManagerTest, MountPath_ReadOnlyDevice) {

[yamaguchi, 2016/09/06 06:21:19]

note: This test function has been merged from head.

   DiskMountManager* manager = DiskMountManager::GetInstance();
   const std::string kSourceFormat = std::string();
   const std::string kMountLabel = std::string();  // N/A for MOUNT_TYPE_DEVICE
 
-  // Event handlers of observers should be called.
-  EXPECT_CALL(
-      observer_,
-      OnMountEvent(DiskMountManager::MOUNTING, chromeos::MOUNT_ERROR_NONE,
-                   Field(&DiskMountManager::MountPointInfo::mount_path,
-                         kReadOnlyMountpath)));
-
   // Attempt to mount a read-only device in read-write mode.
   manager->MountPath(kReadOnlyDeviceSource, kSourceFormat, std::string(),
                      chromeos::MOUNT_TYPE_DEVICE,
                      chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
   // Simulate cros_disks reporting mount completed.
   fake_cros_disks_client_->SendMountCompletedEvent(
       chromeos::MOUNT_ERROR_NONE, kReadOnlyDeviceSource,
       chromeos::MOUNT_TYPE_DEVICE, kReadOnlyMountpath);
 
+  // Event handlers of observers should be called.
+  ASSERT_GE(1U, observer_->GetEventCount());
+  VerifyMountEvent(observer_->GetMountEvent(0), DiskMountManager::MOUNTING,
+                   chromeos::MOUNT_ERROR_NONE, kReadOnlyMountpath);
   const DiskMountManager::DiskMap& disks = manager->disks();
   ASSERT_GT(disks.count(kReadOnlyDeviceSource), 0U);
   // The mounted disk should preserve the read-only flag of the block device.
   EXPECT_TRUE(disks.find(kReadOnlyDeviceSource)->second->is_read_only());
 }
 
 }  // namespace