Integrated test with the actual QuotaManager and PathManager.

webkit/fileapi/file_system_quota_unittest.cc

Index: webkit/fileapi/file_system_quota_unittest.cc
diff --git a/webkit/fileapi/file_system_quota_unittest.cc b/webkit/fileapi/file_system_quota_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a151f9949418a55d53d9811daad0d3cadaef251e
--- /dev/null
+++ b/webkit/fileapi/file_system_quota_unittest.cc
@@ -0,0 +1,378 @@
+// Copyright (c) 2011 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.
+
+// This test checks the entire behavior of FileSystem usage and quota, such as:
+// 1) the actual size of files on disk,
+// 2) the described size in .usage, and
+// 3) the result of QuotaManager::GetUsageAndQuota.
+
+#include "webkit/fileapi/file_system_operation.h"
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/memory/scoped_callback_factory.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/platform_file.h"
+#include "base/scoped_temp_dir.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/fileapi/file_system_callback_dispatcher.h"
+#include "webkit/fileapi/file_system_operation.h"
+#include "webkit/fileapi/file_system_test_helper.h"
+#include "webkit/fileapi/file_system_usage_cache.h"
+#include "webkit/fileapi/file_system_util.h"
+#include "webkit/fileapi/local_file_system_file_util.h"
+#include "webkit/quota/quota_manager.h"
+
+namespace fileapi {
+
+const int kFileOperationStatusNotSet = 1;
+
+class FileSystemQuotaTest : public testing::Test {
+ public:
+  FileSystemQuotaTest()
+      : callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+        status_(kFileOperationStatusNotSet),
+        quota_status_(quota::kQuotaStatusUnknown),
+        usage_(-1),
+        quota_(-1) {}
+
+  FileSystemOperation* operation();
+
+  void set_local_path(const FilePath& path) { local_path_ = path; }
+  const FilePath& local_path() const { return local_path_; }
+  void set_status(int status) { status_ = status; }
+  int status() const { return status_; }
+  void set_info(const base::PlatformFileInfo& info) { info_ = info; }
+  const base::PlatformFileInfo& info() const { return info_; }
+  void set_path(const FilePath& path) { path_ = path; }
+  const FilePath& path() const { return path_; }
+  void set_entries(const std::vector<base::FileUtilProxy::Entry>& entries) {
+    entries_ = entries;
+  }
+  const std::vector<base::FileUtilProxy::Entry>& entries() const {
+    return entries_;
+  }

[kinuko, 2011/06/02 08:04:08]

Many of them seem to be unnecessary (see the other comment for DidXxx in
dispatcher)

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+
+  virtual void SetUp();
+  virtual void TearDown();
+
+  void OnGetUsageAndQuota(
+      quota::QuotaStatusCode status, int64 usage, int64 quota);
+
+ protected:
+  base::PlatformFile OpenFile(const FilePath& virtual_path);
+  void PrepareFileSet(const FilePath& virtual_path);
+
+  GURL URLForPath(const FilePath& path) const {
+    return test_helper_.GetURLForPath(path);
+  }
+
+  FilePath PlatformPath(FilePath virtual_path) {
+    return test_helper_.GetLocalPath(virtual_path);
+  }
+
+  int64 ActualSize() {
+    return file_util::ComputeDirectorySize(base_dir_path_);
+  }
+
+  int64 SizeInUsageFile() {
+    return FileSystemUsageCache::GetUsage(usage_file_path_);
+  }

[kinuko, 2011/06/02 08:04:08]

For these two methods (ActualSize and SizeInUsageFile) the test helper provides
two helper methods:

  int64 GetCachedOriginUsage() const;
  int64 ComputeCurrentOriginUsage() const;

Note that ComputeCurrentOriginUsage() always returns the size that doesn't
include usage file size (for backward compat) so you'll want to drop '+
usage_file_size' from all the lines you do something like
EXPECT_EQ(foo + usage_file_size, ActualSize())

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+
+  int64 SizeFromQuotaManager() {
+    quota_manager_->GetUsageAndQuota(
+        test_helper_.origin(), test_helper_.storage_type(),
+        callback_factory_.NewCallback(
+            &FileSystemQuotaTest::OnGetUsageAndQuota));
+    MessageLoop::current()->RunAllPending();
+    return usage_;
+  }
+
+  bool VirtualFileExists(FilePath virtual_path) {
+    return file_util::PathExists(PlatformPath(virtual_path)) &&
+        !file_util::DirectoryExists(PlatformPath(virtual_path));
+  }
+
+  bool VirtualDirectoryExists(FilePath virtual_path) {
+    return file_util::DirectoryExists(PlatformPath(virtual_path));
+  }
+
+  FilePath CreateVirtualDirectory(const char* virtual_path_string) {
+    FilePath virtual_path(virtual_path_string);
+    file_util::CreateDirectory(PlatformPath(virtual_path));
+    return virtual_path;
+  }
+
+  FilePath CreateVirtualDirectoryInDir(const char* virtual_path_string,
+                                       const FilePath& virtual_dir_path) {
+    FilePath virtual_path(virtual_dir_path.AppendASCII(virtual_path_string));
+    file_util::CreateDirectory(PlatformPath(virtual_path));
+    return virtual_path;
+  }
+
+  FilePath CreateVirtualTemporaryFileInDir(const FilePath& virtual_dir_path) {
+    FilePath absolute_dir_path(PlatformPath(virtual_dir_path));
+    FilePath absolute_file_path;
+    if (file_util::CreateTemporaryFileInDir(absolute_dir_path,
+                                            &absolute_file_path))
+      return virtual_dir_path.Append(absolute_file_path.BaseName());
+    else
+      return FilePath();

[kinuko, 2011/06/02 08:04:08]

For these checks (checking the return value of file_util::CreateTemporary... and
returning empty path if it was false) can we simply call EXPECT_xxx for the
return value of file_util::CreateTemporary?

Returning empty path doesn't look very helpful to me.

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  }
+
+  FilePath CreateVirtualTemporaryDirInDir(const FilePath& virtual_dir_path,
+                                          const FilePath::StringType& prefix) {
+    FilePath absolute_parent_dir_path(PlatformPath(virtual_dir_path));
+    FilePath absolute_child_dir_path;
+    if (file_util::CreateTemporaryDirInDir(absolute_parent_dir_path, prefix,
+                                           &absolute_child_dir_path))
+      return virtual_dir_path.Append(absolute_child_dir_path.BaseName());
+    else
+      return FilePath();
+  }
+
+  FilePath CreateVirtualTemporaryDir(const FilePath::StringType& prefix) {
+    return CreateVirtualTemporaryDirInDir(FilePath(), prefix);
+  }
+
+  FileSystemTestOriginHelper test_helper_;
+
+  ScopedTempDir work_dir_;
+  scoped_refptr<quota::QuotaManager> quota_manager_;
+
+  FilePath filesystem_dir_path_;

[kinuko, 2011/06/02 08:04:08]

Looks like this (and probably two other paths below) doesn't need to be member
variables.

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  FilePath base_dir_path_;
+  FilePath usage_file_path_;
+
+  FilePath child_dir_path_;
+  FilePath child_file1_path_;
+  FilePath child_file2_path_;
+  FilePath grandchild_file1_path_;
+  FilePath grandchild_file2_path_;
+  base::PlatformFile child_file1_;
+  base::PlatformFile child_file2_;
+  base::PlatformFile grandchild_file1_;
+  base::PlatformFile grandchild_file2_;
+
+  base::ScopedCallbackFactory<FileSystemQuotaTest> callback_factory_;
+
+  // For post-operation status.
+  int status_;
+  quota::QuotaStatusCode quota_status_;
+  int64 usage_;
+  int64 quota_;
+  base::PlatformFileInfo info_;
+  FilePath path_;
+  FilePath local_path_;
+  std::vector<base::FileUtilProxy::Entry> entries_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileSystemQuotaTest);
+};
+
+namespace {
+
+class MockDispatcher : public FileSystemCallbackDispatcher {
+ public:
+  MockDispatcher(FileSystemQuotaTest* test) : test_(test) { }
+
+  virtual void DidFail(base::PlatformFileError status) {
+    test_->set_status(status);
+  }
+
+  virtual void DidSucceed() {
+    test_->set_status(base::PLATFORM_FILE_OK);
+  }
+
+  virtual void DidGetLocalPath(const FilePath& local_path) {

[kinuko, 2011/06/02 08:04:08]

We no longer have this method

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+    test_->set_local_path(local_path);
+    test_->set_status(base::PLATFORM_FILE_OK);
+  }
+
+  virtual void DidReadMetadata(
+      const base::PlatformFileInfo& info,
+      const FilePath& platform_path) {
+    test_->set_info(info);
+    test_->set_path(platform_path);
+    test_->set_status(base::PLATFORM_FILE_OK);

[kinuko, 2011/06/02 08:04:08]

this doesn't seem to be used

ADD_FAILURE()

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  }
+
+  virtual void DidReadDirectory(
+      const std::vector<base::FileUtilProxy::Entry>& entries,
+      bool /* has_more */) {
+    test_->set_entries(entries);

[kinuko, 2011/06/02 08:04:08]

this doesn't seem to be used

ADD_FAILURE()

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  }
+
+  virtual void DidOpenFileSystem(const std::string&, const GURL&) {
+    ADD_FAILURE();
+  }
+
+  virtual void DidWrite(int64 bytes, bool complete) {
+    ADD_FAILURE();
+  }
+
+ private:
+  FileSystemQuotaTest* test_;
+};
+
+}  // namespace (anonymous)
+
+void FileSystemQuotaTest::SetUp() {
+  ASSERT_TRUE(work_dir_.CreateUniqueTempDir());
+  filesystem_dir_path_ = work_dir_.path().AppendASCII("filesystem");
+  file_util::CreateDirectory(filesystem_dir_path_);
+
+  quota_manager_ = new quota::QuotaManager(
+      false /* is_incognito */,
+      filesystem_dir_path_,
+      base::MessageLoopProxy::CreateForCurrentThread(),
+      base::MessageLoopProxy::CreateForCurrentThread(),
+      NULL);
+
+  test_helper_.SetUp(filesystem_dir_path_,
+                     false /* incognito */,
+                     false /* unlimited quota */,
+                     quota_manager_->proxy(),
+                     LocalFileSystemFileUtil::GetInstance());
+  base_dir_path_ = test_helper_.GetOriginRootPath();
+  usage_file_path_ = test_helper_.GetUsageCachePath();
+  file_util::Delete(usage_file_path_, false);

[kinuko, 2011/06/02 08:04:08]

This doesn't look right to me...?

If it's for making the computed directory size right, the issue should be
resolved if you use test_helper_ methods to get cached usage size / actual size.

Or if it's for reseting the usage cache I think we should have a method like
ResetUsageCache() in the test helper and call that explicitly in the test code. 
(Wherever before you're checking EXPECT_EQ(-1, SizeInUsageFile()))

I want to make the amount of code code that directly modifies usage cache file
as minimal as possible, so that whenever we want to change the usage cache code
(e.g. we may want to use levelDb to store the usage cache instead of a plain
file) the changes we need to make will be small.

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Removing .usage was required so that .usage catches up to the current disk
state.  Finally, changed the testing procedure as follows.
* Truncating files with operation()->Truncate() without resetting.
* all_file_size (w/o usage_file_size) for ActualSize().
* all_file_size (w/o usage_file_size) for SizeInUsageFile().

+}
+
+void FileSystemQuotaTest::TearDown() {
+  quota_manager_ = NULL;
+
+  // Runs cleanup tasks posted by QuataManager and FileSystemContext.
+  MessageLoop::current()->RunAllPending();
+  test_helper_.TearDown();
+}
+
+FileSystemOperation* FileSystemQuotaTest::operation() {
+  return test_helper_.NewOperation(new MockDispatcher(this));
+}
+
+base::PlatformFile FileSystemQuotaTest::OpenFile(const FilePath& virtual_path) {
+  base::PlatformFile file;
+  bool created;
+  base::PlatformFileError error_code;
+  file = base::CreatePlatformFile(
+      PlatformPath(virtual_path),
+      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE |
+          base::PLATFORM_FILE_ASYNC, &created, &error_code);

[kinuko, 2011/06/02 08:04:08]

maybe better to check error_code here?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  return file;
+}
+
+void FileSystemQuotaTest::OnGetUsageAndQuota(
+    quota::QuotaStatusCode status, int64 usage, int64 quota) {
+  quota_status_ = status;
+  usage_ = usage;
+  quota_ = quota;
+}
+
+void FileSystemQuotaTest::PrepareFileSet(const FilePath& virtual_path) {
+  child_dir_path_ = CreateVirtualTemporaryDirInDir(virtual_path, "prefix");
+  child_file1_path_ = CreateVirtualTemporaryFileInDir(virtual_path);
+  child_file1_ = OpenFile(child_file1_path_);
+  child_file2_path_ = CreateVirtualTemporaryFileInDir(virtual_path);
+  child_file2_ = OpenFile(child_file2_path_);
+  grandchild_file1_path_ = CreateVirtualTemporaryFileInDir(child_dir_path_);
+  grandchild_file1_ = OpenFile(grandchild_file1_path_);
+  grandchild_file2_path_ = CreateVirtualTemporaryFileInDir(child_dir_path_);
+  grandchild_file2_ = OpenFile(grandchild_file2_path_);

[kinuko, 2011/06/02 08:04:08]

Don't we need to close the opened files in TearDown?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+}
+
+TEST_F(FileSystemQuotaTest, TestMoveSuccessSrcDirRecursive) {
+  FilePath src_dir_path(CreateVirtualTemporaryDir(""));
+  PrepareFileSet(src_dir_path);
+  FilePath dest_dir_path(CreateVirtualTemporaryDir(""));
+
+  EXPECT_EQ(0, ActualSize());
+
+  ASSERT_TRUE(base::TruncatePlatformFile(child_file1_, 1013));
+  ASSERT_TRUE(base::TruncatePlatformFile(child_file2_, 129));
+  ASSERT_TRUE(base::TruncatePlatformFile(grandchild_file1_, 94));
+  ASSERT_TRUE(base::TruncatePlatformFile(grandchild_file2_, 517));

[kinuko, 2011/06/02 08:04:08]

nit: these days I'm trying to use simpler values like 1, 2, 3... or 10, 200,
3000, ... for test values, since if we do so we can instantly know what went
wrong from the test output log.

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Changed it to { 5000, 400, 30, 2 } and { 8000, 700, 60, 5 }.

+

[kinuko, 2011/06/02 08:04:08]

it might be better to check we have enough quota here (e.g. ASSERT_GT(quota(),
all_file_size) etc)

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  int64 child_file_size = 1013+129;
+  int64 grandchild_file_size = 94+517;
+  int64 all_file_size = child_file_size + grandchild_file_size;

[kinuko, 2011/06/02 08:04:08]

nit: For this test seems like you can directly calculate the all_file_size
(without two mostly unsed variables (i.e. child_ and grandchild_))

[kinuko, 2011/06/02 08:04:08]

nit: const ?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+  int64 usage_file_size = FileSystemUsageCache::kUsageFileSize;

[kinuko, 2011/06/02 08:04:08]

nit: const ?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+
+  EXPECT_EQ(all_file_size, ActualSize());
+  EXPECT_EQ(-1, SizeInUsageFile());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeFromQuotaManager());
+  EXPECT_EQ(all_file_size + usage_file_size, ActualSize());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeInUsageFile());
+
+  operation()->Move(URLForPath(src_dir_path), URLForPath(dest_dir_path));
+  MessageLoop::current()->RunAllPending();

[kinuko, 2011/06/02 08:04:08]

should check status_ here?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done in the following line.

+
+  EXPECT_EQ(base::PLATFORM_FILE_OK, status());
+  EXPECT_TRUE(VirtualDirectoryExists(dest_dir_path.Append(
+      child_dir_path_.BaseName())));
+  EXPECT_TRUE(VirtualFileExists(dest_dir_path.Append(
+      child_dir_path_.BaseName()).Append(
+      grandchild_file1_path_.BaseName())));
+
+  EXPECT_EQ(all_file_size + usage_file_size, ActualSize());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeInUsageFile());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeFromQuotaManager());
+}
+
+TEST_F(FileSystemQuotaTest, TestCopySuccessSrcDirRecursive) {
+  FilePath src_dir_path(CreateVirtualTemporaryDir(""));
+  PrepareFileSet(src_dir_path);
+  FilePath dest_dir1_path(CreateVirtualTemporaryDir(""));
+  FilePath dest_dir2_path(CreateVirtualTemporaryDir(""));
+
+  EXPECT_EQ(0, ActualSize());
+
+  ASSERT_TRUE(base::TruncatePlatformFile(child_file1_, 12));
+  ASSERT_TRUE(base::TruncatePlatformFile(child_file2_, 23));
+  ASSERT_TRUE(base::TruncatePlatformFile(grandchild_file1_, 34));
+  ASSERT_TRUE(base::TruncatePlatformFile(grandchild_file2_, 45));
+
+  int64 child_file_size = 12+23;
+  int64 grandchild_file_size = 34+45;
+  int64 all_file_size = child_file_size + grandchild_file_size;
+  int64 usage_file_size = FileSystemUsageCache::kUsageFileSize;
+
+  EXPECT_EQ(all_file_size, ActualSize());
+  EXPECT_EQ(-1, SizeInUsageFile());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeFromQuotaManager());
+  EXPECT_EQ(all_file_size + usage_file_size, ActualSize());
+  EXPECT_EQ(all_file_size + usage_file_size, SizeInUsageFile());
+
+  operation()->Copy(URLForPath(src_dir_path), URLForPath(dest_dir1_path));
+  MessageLoop::current()->RunAllPending();
+
+  EXPECT_EQ(base::PLATFORM_FILE_OK, status());
+  EXPECT_TRUE(VirtualDirectoryExists(src_dir_path.Append(
+      child_dir_path_.BaseName())));
+  EXPECT_TRUE(VirtualFileExists(src_dir_path.Append(
+      child_dir_path_.BaseName()).Append(
+      grandchild_file1_path_.BaseName())));
+  EXPECT_EQ(base::PLATFORM_FILE_OK, status());
+  EXPECT_TRUE(VirtualDirectoryExists(dest_dir1_path.Append(
+      child_dir_path_.BaseName())));
+  EXPECT_TRUE(VirtualFileExists(dest_dir1_path.Append(
+      child_dir_path_.BaseName()).Append(
+      grandchild_file1_path_.BaseName())));
+
+  EXPECT_EQ(2 * all_file_size + usage_file_size, ActualSize());
+  EXPECT_EQ(2 * all_file_size + usage_file_size, SizeInUsageFile());
+  EXPECT_EQ(2 * all_file_size + usage_file_size, SizeFromQuotaManager());
+
+  operation()->Copy(URLForPath(child_dir_path_), URLForPath(dest_dir2_path));
+  MessageLoop::current()->RunAllPending();

[kinuko, 2011/06/02 08:04:08]

should check status_ here?

[Dai Mikurube (NOT FULLTIME), 2011/06/13 08:12:09]

Done.

+
+  EXPECT_EQ(2 * child_file_size + 3 * grandchild_file_size + usage_file_size,
+      ActualSize());
+  EXPECT_EQ(2 * child_file_size + 3 * grandchild_file_size + usage_file_size,
+      SizeInUsageFile());
+  EXPECT_EQ(2 * child_file_size + 3 * grandchild_file_size + usage_file_size,
+      SizeFromQuotaManager());
+}
+
+}  // namespace fileapi