drive: Stop using FilePathWatcher from FileManagerBrowserTest

chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js

Index: chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js b/chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
index b4f5352c4a833d57def58fec4c840043804b0c91..6df2569e1a4c9f2a7a72c855f66d82e057cca65c 100644
--- a/chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
+++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/test_cases.js
@@ -116,6 +116,33 @@ var EXPECTED_FILES_IN_SHARED_WITH_ME = [
 ];
 
 /**
+ * Returns the name of the given file list entry.
+ * @param {Array.<string>} file An entry in a flie list.
+ * @return {string} Name of the file.
+ */
+function getFileName(fileListEntry) {
+  return fileListEntry[0];
+}
+
+/**
+ * Returns the size of the given file list entry.
+ * @param {Array.<string>} An entry in a flie list.

[mtomasz, 2013/05/28 02:33:41]

nit: flie -> file

[hashimoto, 2013/05/28 05:08:12]

Done for all three occurrences.

+ * @return {string} Size of the file.
+ */
+function getFileSize(fileListEntry) {
+  return fileListEntry[1];
+}
+
+/**
+ * Returns the type of the given file list entry.
+ * @param {Array.<string>} An entry in a flie list.
+ * @return {string} Type of the file.
+ */
+function getFileType(fileListEntry) {
+  return fileListEntry[2];
+}
+
+/**
  * Namespace for test cases.
  */
 var testcase = {};
@@ -221,14 +248,46 @@ testcase.intermediate.galleryOpen = function(path) {
  * @param {string} path Directory path to be tested.
  */
 testcase.intermediate.keyboardCopy = function(path, callback) {
-  setupAndWaitUntilReady(path, function(appId) {
-    callRemoteTestUtil('copyFile', appId, ['world.ogv'], function(result) {
-      chrome.test.assertFalse(!result);
-      callRemoteTestUtil('waitForFileListChange',
-                         appId,
-                         [getExpectedFilesBefore(path == '/drive/root').length],
-                         checkIfNoErrorsOccured.bind(null,
-                                                     chrome.test.succeed));
+  setupAndWaitUntilReady(path, function(appId, files) {
+    // Returns true if |fileList| contains a copy of |filename|.
+    function isCopyPresent(filename, fileList) {

[mtomasz, 2013/05/28 02:33:41]

Inline functions are not recommended in Files.app.
Should be: var isCopyPresent = function(filename, fileList).
I know we have some in test_util.js, but we shouldn't introduce new.

[hashimoto, 2013/05/28 05:08:12]

Done.

+      var original_entry;
+      for (var i = 0; i < fileList.length; ++i) {

[mtomasz, 2013/05/28 02:33:41]

In Files.app i++ is much more common than ++i.
++i: 12 times
i++: 131 times.
Let's be consistent.

[hashimoto, 2013/05/28 05:08:12]

Done for all three.

+        if (getFileName(fileList[i]) == filename)
+          original_entry = fileList[i];
+      }
+      if (!original_entry)
+        return false;
+
+      var base_name = filename.substring(0, filename.lastIndexOf('.'));
+      var extension = filename.substr(filename.lastIndexOf('.'));
+      var filename_pattern =
+          new RegExp('^' + base_name + '.+' + extension + '$');

[mtomasz, 2013/05/28 02:33:41]

in JS camel case: baseName instead of base_name.

[hashimoto, 2013/05/28 05:08:12]

Done.

+      for (var i = 0; i < fileList.length; ++i) {
+        // Check size, type and file name pattern to find a copy.
+        if (getFileSize(fileList[i]) == getFileSize(original_entry) &&
+            getFileType(fileList[i]) == getFileType(original_entry) &&
+            filename_pattern.exec(getFileName(fileList[i])))
+          return true;
+      }
+      return false;
+    }
+
+    var filename = 'world.ogv';
+    chrome.test.assertFalse(isCopyPresent(filename, files));
+
+    callRemoteTestUtil('copyFile', appId, [filename], function(result) {
+      chrome.test.assertTrue(result);

[mtomasz, 2013/05/28 02:33:41]

How about calling waitForFileListChange and then assertTrue(isCopyPresent)? This
would simplify and make if faster in case of failures.

[hashimoto, 2013/05/28 05:08:12]

Done.

+      function waitForCopyCompletion() {
+        callRemoteTestUtil('getFileList', appId, [], function(fileList) {
+          if (isCopyPresent(filename, fileList)) {
+            checkIfNoErrorsOccured(chrome.test.succeed);
+            return;
+          }
+          window.setTimeout(waitForCopyCompletion, 50);
+        });
+      }
+      waitForCopyCompletion();
     });
   });
 };
@@ -238,15 +297,32 @@ testcase.intermediate.keyboardCopy = function(path, callback) {
  * @param {string} path Directory path to be tested.
  */
 testcase.intermediate.keyboardDelete = function(path) {
-  setupAndWaitUntilReady(path, function(appId) {
-    callRemoteTestUtil('deleteFile', appId, ['world.ogv'], function(result) {
-      chrome.test.assertFalse(!result);
+  setupAndWaitUntilReady(path, function(appId, files) {

[mtomasz, 2013/05/28 02:33:41]

@hirono introduced a great pattern for complex multi-step tests. Please take a
look at copyBetweenVolumes. Can we use it here?

[hashimoto, 2013/05/28 05:08:12]

Done.

+    // Returns true if |fileList| contains |filename|.
+    function isFilePresent(filename, fileList) {
+      for (var i = 0; i < fileList.length; ++i) {
+        if (getFileName(fileList[i]) == filename)
+          return true;
+      }
+      return false;
+    }
+
+    var filename = 'world.ogv';
+    chrome.test.assertTrue(isFilePresent(filename, files));
+
+    callRemoteTestUtil('deleteFile', appId, [filename], function(result) {
+      chrome.test.assertTrue(result);
       callRemoteTestUtil('waitAndAcceptDialog', appId, [], function() {
-        callRemoteTestUtil(
-            'waitForFileListChange',
-            appId,
-            [getExpectedFilesBefore(path == '/drive/root').length],
-            checkIfNoErrorsOccured.bind(null, chrome.test.succeed));
+        function waitForDeleteCompletion() {

[mtomasz, 2013/05/28 02:33:41]

I think we can use waitForFileListChange here for simpler code.

[hashimoto, 2013/05/28 05:08:12]

Done.

+          callRemoteTestUtil('getFileList', appId, [], function(fileList) {
+            if (!isFilePresent(filename, fileList)) {
+              checkIfNoErrorsOccured(chrome.test.succeed);
+              return;
+            }
+            window.setTimeout(waitForDeleteCompletion, 50);
+          });
+        }
+        waitForDeleteCompletion();
       });
     });
   });