Rewrite DownloadHandlerTest using DummyFileSystem.

chrome/browser/chromeos/drive/download_handler_unittest.cc

Index: chrome/browser/chromeos/drive/download_handler_unittest.cc
diff --git a/chrome/browser/chromeos/drive/download_handler_unittest.cc b/chrome/browser/chromeos/drive/download_handler_unittest.cc
index 5fd48d3a54e176e414be0664a03d66af994eb333..9c8bd07fe43cd18c225cfcd1ebde454c68cf3630 100644
--- a/chrome/browser/chromeos/drive/download_handler_unittest.cc
+++ b/chrome/browser/chromeos/drive/download_handler_unittest.cc
@@ -6,9 +6,9 @@
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop.h"
+#include "chrome/browser/chromeos/drive/dummy_file_system.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/chromeos/drive/file_write_helper.h"
-#include "chrome/browser/chromeos/drive/mock_file_system.h"
 #include "chrome/browser/google_apis/test_util.h"
 #include "content/public/test/mock_download_item.h"
 #include "content/public/test/mock_download_manager.h"
@@ -16,25 +16,53 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using ::testing::Return;
-using ::testing::SaveArg;
-using ::testing::_;
-
 namespace drive {
 
 namespace {
 
-// Copies |file_path| to |out_file_path|, is used as a
-// SubstituteDriveDownloadPathCallback.
-void CopySubstituteDriveDownloadPathResult(base::FilePath* out_file_path,
-                                           const base::FilePath& file_path) {
-  *out_file_path = file_path;
-}
+// Flags to control the state of the test file system.
+enum DownloadPathState {
+  // Simulates the state that requested path just simply exists.
+  PATH_EXISTS,
+  // Simulates the state that requested path fails to be accessed.
+  PATH_INVALID,
+  // Simulates the state that the requested path does not exist.
+  PATH_NOT_EXIST,
+  // Simulates the state that the path does not exist nor be able to be created.
+  PATH_NOT_EXIST_AND_CREATE_FAIL,
+};
 
-// Copies |value| to |out|, is used as a content::CheckForFileExistenceCallback.
-void CopyCheckForFileExistenceResult(bool* out, bool value) {
-  *out = value;
-}
+// Test file system to

[hashimoto, 2013/06/04 00:53:09]

nit: "Test file system to DownloadHandlerTest"?

[kinaba, 2013/06/04 03:46:26]

Done.

+class DownloadHandlerTestFileSystem : public DummyFileSystem {
+ public:
+  DownloadHandlerTestFileSystem() : state_(PATH_INVALID) {}
+
+  void set_download_path_state(DownloadPathState state) { state_ = state; }
+
+  // FileSystemInterface overrides.
+  virtual void GetResourceEntryByPath(
+      const base::FilePath& file_path,
+      const GetResourceEntryCallback& callback) OVERRIDE {
+    if (state_ == PATH_EXISTS) {
+      callback.Run(FILE_ERROR_OK, make_scoped_ptr(new ResourceEntry));
+      return;
+    }
+    callback.Run(
+        state_ == PATH_INVALID ? FILE_ERROR_FAILED : FILE_ERROR_NOT_FOUND,
+        scoped_ptr<ResourceEntry>());
+ }
+
+ virtual void CreateDirectory(
+     const base::FilePath& directory_path,
+     bool is_exclusive,
+     bool is_recursive,
+     const FileOperationCallback& callback) OVERRIDE {
+   callback.Run(state_ == PATH_NOT_EXIST ? FILE_ERROR_OK : FILE_ERROR_FAILED);
+ }
+
+ private:
+  DownloadPathState state_;
+};
 
 }  // namespace
 
@@ -49,36 +77,23 @@ class DownloadHandlerTest : public testing::Test {
 
     // Set expectations for download item.
     EXPECT_CALL(download_item_, GetState())
-        .WillRepeatedly(Return(content::DownloadItem::IN_PROGRESS));
-
-    // Set expectations for file system to save argument callbacks.
-    EXPECT_CALL(file_system_, GetResourceEntryByPath(_, _))
-        .WillRepeatedly(SaveArg<1>(&get_entry_info_callback_));
-    EXPECT_CALL(file_system_, CreateDirectory(_, _, _, _))
-        .WillRepeatedly(SaveArg<3>(&create_directory_callback_));
+        .WillRepeatedly(testing::Return(content::DownloadItem::IN_PROGRESS));

[hashimoto, 2013/06/04 00:53:09]

nit: Can't we make this line unchanged?

[kinaba, 2013/06/04 03:46:26]

As talked offline, let's keep it in the changed form (to reduce the number of
"using"s).

 
-    file_write_helper_.reset(new FileWriteHelper(&file_system_));
+    file_write_helper_.reset(new FileWriteHelper(&test_file_system_));
     download_handler_.reset(
-        new DownloadHandler(file_write_helper_.get(), &file_system_));
+        new DownloadHandler(file_write_helper_.get(), &test_file_system_));
     download_handler_->Initialize(download_manager_.get(), temp_dir_.path());
   }
 
-  virtual void TearDown() OVERRIDE {
-  }
-
  protected:
   base::ScopedTempDir temp_dir_;
   base::MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
   scoped_ptr<content::MockDownloadManager> download_manager_;
-  MockFileSystem file_system_;
+  DownloadHandlerTestFileSystem test_file_system_;
   scoped_ptr<FileWriteHelper> file_write_helper_;
   scoped_ptr<DownloadHandler> download_handler_;
   content::MockDownloadItem download_item_;
-
-  // Argument callbacks passed to the file system.
-  GetResourceEntryCallback get_entry_info_callback_;
-  FileOperationCallback create_directory_callback_;
 };
 
 TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPathNonDrivePath) {
@@ -90,7 +105,7 @@ TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPathNonDrivePath) {
   download_handler_->SubstituteDriveDownloadPath(
       non_drive_path,
       &download_item_,
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
@@ -102,18 +117,15 @@ TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPath) {
   const base::FilePath drive_path =
       util::GetDriveMountPointPath().AppendASCII("test.dat");
 
+  // Test the case that the download target directory already exists.
+  test_file_system_.set_download_path_state(PATH_EXISTS);
+
   // Call SubstituteDriveDownloadPath()
   base::FilePath substituted_path;
   download_handler_->SubstituteDriveDownloadPath(
       drive_path,
       &download_item_,
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), destination directory found.
-  scoped_ptr<ResourceEntry> entry(new ResourceEntry);
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_OK, entry.Pass());
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
@@ -126,18 +138,16 @@ TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPathGetEntryFailure) {
   const base::FilePath drive_path =
       util::GetDriveMountPointPath().AppendASCII("test.dat");
 
+  // Test the case that access to the download target directory failed for some
+  // reason.
+  test_file_system_.set_download_path_state(PATH_INVALID);
+
   // Call SubstituteDriveDownloadPath()
   base::FilePath substituted_path;
   download_handler_->SubstituteDriveDownloadPath(
       drive_path,
       &download_item_,
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), failing for some reason.
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_FAILED,
-                              scoped_ptr<ResourceEntry>());
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
@@ -148,23 +158,16 @@ TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPathCreateDirectory) {
   const base::FilePath drive_path =
       util::GetDriveMountPointPath().AppendASCII("test.dat");
 
+  // Test the case that access to the download target directory does not exist,
+  // and thus will be created in DownloadHandler.
+  test_file_system_.set_download_path_state(PATH_NOT_EXIST);
+
   // Call SubstituteDriveDownloadPath()
   base::FilePath substituted_path;
   download_handler_->SubstituteDriveDownloadPath(
       drive_path,
       &download_item_,
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), destination directory not found.
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_NOT_FOUND,
-                              scoped_ptr<ResourceEntry>());
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of CreateDirecotry().
-  ASSERT_FALSE(create_directory_callback_.is_null());
-  create_directory_callback_.Run(FILE_ERROR_OK);
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
@@ -178,23 +181,16 @@ TEST_F(DownloadHandlerTest,
   const base::FilePath drive_path =
       util::GetDriveMountPointPath().AppendASCII("test.dat");
 
+  // Test the case that access to the download target directory does not exist,
+  // and creation fails for some reason.
+  test_file_system_.set_download_path_state(PATH_NOT_EXIST_AND_CREATE_FAIL);
+
   // Call SubstituteDriveDownloadPath()
   base::FilePath substituted_path;
   download_handler_->SubstituteDriveDownloadPath(
       drive_path,
       &download_item_,
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), destination directory not found.
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_NOT_FOUND,
-                              scoped_ptr<ResourceEntry>());
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of CreateDirecotry().
-  ASSERT_FALSE(create_directory_callback_.is_null());
-  create_directory_callback_.Run(FILE_ERROR_FAILED);
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
@@ -206,19 +202,14 @@ TEST_F(DownloadHandlerTest,
 TEST_F(DownloadHandlerTest, SubstituteDriveDownloadPathForSavePackage) {
   const base::FilePath drive_path =
       util::GetDriveMountPointPath().AppendASCII("test.dat");
+  test_file_system_.set_download_path_state(PATH_EXISTS);
 
   // Call SubstituteDriveDownloadPath()
   base::FilePath substituted_path;
   download_handler_->SubstituteDriveDownloadPath(
       drive_path,
       NULL,  // DownloadItem is not available at this moment.
-      base::Bind(&CopySubstituteDriveDownloadPathResult, &substituted_path));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), destination directory found.
-  scoped_ptr<ResourceEntry> entry(new ResourceEntry);
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_OK, entry.Pass());
+      google_apis::test_util::CreateCopyResultCallback(&substituted_path));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result of SubstituteDriveDownloadPath().
@@ -244,37 +235,26 @@ TEST_F(DownloadHandlerTest, CheckForFileExistence) {
   ASSERT_TRUE(download_handler_->IsDriveDownload(&download_item_));
   EXPECT_EQ(drive_path, download_handler_->GetTargetPath(&download_item_));
 
+  // Test for the case when the path exists.
+  test_file_system_.set_download_path_state(PATH_EXISTS);
+
   // Call CheckForFileExistence.
   bool file_exists = false;
   download_handler_->CheckForFileExistence(
       &download_item_,
-      base::Bind(&CopyCheckForFileExistenceResult, &file_exists));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), file exists.
-  {
-    scoped_ptr<ResourceEntry> entry(new ResourceEntry);
-    ASSERT_FALSE(get_entry_info_callback_.is_null());
-    get_entry_info_callback_.Run(FILE_ERROR_OK, entry.Pass());
-  }
+      google_apis::test_util::CreateCopyResultCallback(&file_exists));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.
   EXPECT_TRUE(file_exists);
 
-  // Reset callback to call CheckForFileExistence again.
-  get_entry_info_callback_.Reset();
+  // Test for the case when the path does not exist.
+  test_file_system_.set_download_path_state(PATH_NOT_EXIST);
 
   // Call CheckForFileExistence again.
   download_handler_->CheckForFileExistence(
       &download_item_,
-      base::Bind(&CopyCheckForFileExistenceResult, &file_exists));
-  google_apis::test_util::RunBlockingPoolTask();
-
-  // Return result of GetResourceEntryByPath(), file does not exist.
-  ASSERT_FALSE(get_entry_info_callback_.is_null());
-  get_entry_info_callback_.Run(FILE_ERROR_NOT_FOUND,
-                               scoped_ptr<ResourceEntry>());
+      google_apis::test_util::CreateCopyResultCallback(&file_exists));
   google_apis::test_util::RunBlockingPoolTask();
 
   // Check the result.