Implements the storage free space watching functionality into storage info provider

chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc

Index: chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc
diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8b7d4424fc809cd84e8928fa15127454abf54396
--- /dev/null
+++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc
@@ -0,0 +1,241 @@
+// 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.
+
+// StorageInfoProvider unit tests.
+
+#include "base/message_loop.h"
+#include "base/run_loop.h"
+#include "chrome/browser/extensions/api/system_info_storage/storage_info_provider.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+using api::experimental_system_info_storage::FromStorageUnitTypeString;
+using api::experimental_system_info_storage::StorageUnitInfo;
+using api::experimental_system_info_storage::StorageUnitType;
+using content::BrowserThread;
+using testing::AnyNumber;
+using testing::Return;
+using testing::_;
+
+struct TestUnitInfo {
+  std::string id;
+  std::string type;
+  double capacity;
+  double available_capacity;
+  // The change step of free space.
+  int change_step;
+};
+
+struct TestUnitInfo testing_data[] = {
+  {"C:", "unknown", 1000, 10, 0},
+  {"d:", "removable", 2000, 10, 1 },
+  {"/home","harddisk", 3000, 10, 2},
+  {"/", "removable", 4000, 10, 3}
+};
+
+// The watching interval for unit test is 100 milliseconds.

[Ilya Sherman, 2012/12/03 19:38:00]

Why is this set so high?  Does something fail if you set the interval to
something more like 1 millisecond?  Setting this to 100 milliseconds, and
waiting for 100's of milliseconds within the test forces each test case to take
100's of milliseconds to run.  Given the number of unit tests that Chrome has,
this is not sustainable; and it doesn't seem necessary for testing the
functionality that you're testing here.

[Hongbo Min, 2012/12/04 02:58:25]

No special consideration on the value. 10 milliseconds might be also fine. 

+const unsigned int kWatchingIntervalMs = 100;
+const int kCallTimes = 3;
+
+class MockStorageObserver : public StorageInfoProvider::Observer {
+ public:
+  MockStorageObserver() {}
+  virtual ~MockStorageObserver() {}
+
+  MOCK_METHOD3(OnStorageFreeSpaceChanged, void(const std::string&,
+     double, double));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockStorageObserver);
+};
+
+class TestStorageInfoProvider : public StorageInfoProvider {
+ public:
+  TestStorageInfoProvider() {}
+  virtual ~TestStorageInfoProvider() {}
+
+ private:
+  virtual bool QueryInfo(StorageInfo* info) OVERRIDE {
+    info->clear();
+
+    for (size_t i = 0; i < arraysize(testing_data); i++) {
+      linked_ptr<StorageUnitInfo> unit(new StorageUnitInfo());
+      QueryUnitInfo(testing_data[i].id, unit.get());
+      info->push_back(unit);
+    }
+    return true;
+  }
+
+  virtual bool QueryUnitInfo(const std::string& id,
+                             StorageUnitInfo* info) OVERRIDE {
+    for (size_t i = 0; i < arraysize(testing_data); i++) {
+      if (testing_data[i].id == id) {
+        info->id = testing_data[i].id;
+        info->type = FromStorageUnitTypeString(testing_data[i].type);
+        info->capacity = testing_data[i].capacity;
+        info->available_capacity = testing_data[i].available_capacity;
+        // Increase the available capacity with a fix change step.
+        testing_data[i].available_capacity += testing_data[i].change_step;
+        return true;
+      }
+    }
+    return false;
+  }
+};
+
+class StorageInfoProviderTest : public testing::Test {
+ public:
+  StorageInfoProviderTest();
+  virtual ~StorageInfoProviderTest();
+
+  MockStorageObserver& observer() { return observer_; }
+
+ protected:
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+  // Run message loop until the given |ms| milliseconds has passed.
+  void RunMessageLoopUntilTimeout(int ms);
+
+  MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+  MockStorageObserver observer_;
+};
+
+StorageInfoProviderTest::StorageInfoProviderTest()
+    : message_loop_(MessageLoop::TYPE_UI),
+      ui_thread_(BrowserThread::UI, &message_loop_),
+      file_thread_(BrowserThread::FILE, &message_loop_) {
+}
+
+StorageInfoProviderTest::~StorageInfoProviderTest() {
+}
+
+void StorageInfoProviderTest::SetUp() {
+  ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  TestStorageInfoProvider* provider = new TestStorageInfoProvider();
+  provider->set_watching_interval(kWatchingIntervalMs);
+  // Now the provider is owned by the singleton instance.
+  StorageInfoProvider::InitializeForTesting(provider);
+  StorageInfoProvider::Get()->AddObserver(&observer_);
+}
+
+void StorageInfoProviderTest::TearDown() {
+  StorageInfoProvider::Get()->RemoveObserver(&observer_);
+}
+
+void StorageInfoProviderTest::RunMessageLoopUntilTimeout(int ms) {
+  base::RunLoop run_loop;
+  message_loop_.PostDelayedTask(FROM_HERE, run_loop.QuitClosure(),
+      base::TimeDelta::FromMilliseconds(ms));
+  run_loop.Run();
+}
+
+TEST_F(StorageInfoProviderTest, WatchingNoChangedStorage) {
+  // Case 1: watching a storage that the free space is not changed.
+  EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[0].id,  _,  _))
+      .Times(0);
+  StorageInfoProvider::Get()->StartWatching(testing_data[0].id);
+  RunMessageLoopUntilTimeout(300);
+  StorageInfoProvider::Get()->StopWatching(testing_data[0].id);
+  RunMessageLoopUntilTimeout(100);
+}
+
+TEST_F(StorageInfoProviderTest, WatchingOneStorage) {
+  // Case 2: only watching one storage.
+  EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id,  _,  _))
+      .WillRepeatedly(Return());
+
+  double base_value = testing_data[1].available_capacity;
+  int step = testing_data[1].change_step;
+  for (int i = 1; i <= kCallTimes; i++) {

[Ilya Sherman, 2012/12/03 19:38:00]

You should use "++i" rather than "i++"

+    double expected_old_value = base_value + i * step;
+    double expected_new_value = base_value + (i + 1) * step;
+    EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id,
+        expected_old_value, expected_new_value)).WillOnce(Return());
+  }
+
+  for (size_t i = 2; i < arraysize(testing_data); ++i) {
+    EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[i].id, _, _))
+        .Times(0);
+  }
+  StorageInfoProvider::Get()->StartWatching(testing_data[1].id);
+  RunMessageLoopUntilTimeout(500);

[Ilya Sherman, 2012/12/03 19:38:00]

You should *never* use timeouts in tests unless you really have no other
recourse.  If you're listening for a timeout, your test will almost always end
up being flaky.  You should wait for an explicit event instead.

[Hongbo Min, 2012/12/04 02:58:25]

Yes, I really have no other recourse. I just a post a delayed task to quit the
message loop after a given time interval since there is no event received for
quitting the message loop in my test. But I am not sure why the tests are always
ending up being flaky.

[Ilya Sherman, 2012/12/04 22:03:55]

At least for the cases that expect an observer to be notified, there is an
explicit event that you can wait for: the observer being notified.  Waiting for
that event might require using custom mocking rather than using gMock, but it's
certainly feasible.

For the cases where you're testing that observers *aren't* notified, it's a
little trickier.  One option is to add a call within
StorageInfoProvider::CheckWatchedStorages() to a method named something like
CheckWatchedStoragesFinishedForTesting(), which is only exposed to the test
code.  You could then wait until that method is called, and then return.

+
+  EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, _, _))
+      .Times(0);
+  StorageInfoProvider::Get()->StopWatching(testing_data[1].id);
+  RunMessageLoopUntilTimeout(200);
+}
+
+TEST_F(StorageInfoProviderTest, WatchingMultipleStorages) {
+  // Case 2: watching multiple storages.
+  for (size_t k = 1; k < arraysize(testing_data); ++k) {

[Ilya Sherman, 2012/12/03 19:38:00]

Why does this loop start at 1 rather than 0?  If this is intentional, the
rationale should be documented with a comment.

[Hongbo Min, 2012/12/04 02:58:25]

Since the change step of the first one is zero. I will update the comment.

+    EXPECT_CALL(observer(),
+        OnStorageFreeSpaceChanged(testing_data[k].id,  _, _))
+        .WillRepeatedly(Return());
+  }
+  for (size_t k = 1; k < arraysize(testing_data); ++k) {
+    double base_value = testing_data[k].available_capacity;
+    int step = testing_data[k].change_step;
+    for (int i = 1; i <= kCallTimes; i++) {
+      double expected_old_value = base_value + i * step;
+      double expected_new_value = base_value + (i + 1) * step;
+      EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id,
+          expected_old_value, expected_new_value)).WillOnce(Return());
+    }
+  }
+  for (size_t k = 1; k < arraysize(testing_data); ++k) {
+    StorageInfoProvider::Get()->StartWatching(testing_data[k].id);
+  }
+  RunMessageLoopUntilTimeout(500);
+
+  // Stop watching the first storage.
+  EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, _, _))
+      .Times(0);
+  for (size_t k = 2; k < arraysize(testing_data); ++k) {
+    EXPECT_CALL(observer(),
+        OnStorageFreeSpaceChanged(testing_data[k].id,  _, _))
+        .WillRepeatedly(Return());
+  }
+  for (size_t k = 2; k < arraysize(testing_data); ++k) {
+    double base_value = testing_data[k].available_capacity;
+    int step = testing_data[k].change_step;
+    for (int i = 1; i <= kCallTimes; i++) {
+      double expected_old_value = base_value + i * step;
+      double expected_new_value = base_value + (i + 1) * step;
+      EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id,
+          expected_old_value, expected_new_value)).WillOnce(Return());
+    }
+  }
+  StorageInfoProvider::Get()->StopWatching(testing_data[1].id);
+  RunMessageLoopUntilTimeout(500);
+
+  for (size_t k = 1; k < arraysize(testing_data); ++k) {
+    EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id, _, _))
+        .Times(0);
+  }
+  // Stop watching all storages.
+  for (size_t k = 1; k < arraysize(testing_data); ++k) {
+    StorageInfoProvider::Get()->StopWatching(testing_data[k].id);
+  }
+  RunMessageLoopUntilTimeout(300);
+}
+
+TEST_F(StorageInfoProviderTest, WatchingInvalidStorage) {
+  // Case 3: watching an invalid storage.
+  std::string invalid_id("invalid_id");
+  EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(invalid_id, _, _))
+     .Times(0);
+  StorageInfoProvider::Get()->StartWatching(invalid_id);
+  RunMessageLoopUntilTimeout(200);
+  StorageInfoProvider::Get()->StopWatching(invalid_id);
+  RunMessageLoopUntilTimeout(100);
+}
+
+} // namespace extensions