drive: Stop using FilePathWatcher from FileManagerBrowserTest

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

 // Copyright (c) 2013 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.
 
 /**
  * Expected files before tests are performed. Entries for Local tests.
  * @type {Array.<Array.<string>>}
  * @const
  */
 var EXPECTED_FILES_BEFORE_LOCAL = [
@@ skipping 98 matching lines @@
  * Expected files shown in "Shared with me", which should be the entries labeled
  * with "shared-with-me".
  * @type {Array.<Array.<string>>}
  * @const
  */
 var EXPECTED_FILES_IN_SHARED_WITH_ME = [
   ['Test Shared Document.gdoc','--','Google document','Mar 20, 2013 10:40 PM']
 ];
 
 /**
+ * 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 = {};
 
 /**
  * Namespace for intermediate test cases.
  * */
 testcase.intermediate = {};
 
 /**
@@ skipping 85 matching lines @@
   steps.shift()();
 };
 
 /**
  * Tests copying a file to the same directory and waits until the file lists
  * changes.
  *
  * @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();
     });
   });
 };
 
 /**
  * Tests deleting a file and and waits until the file lists changes.
  * @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();
       });
     });
   });
 };
 
 testcase.fileDisplayDownloads = function() {
   testcase.intermediate.fileDisplay('/Downloads');
 };
 
 testcase.galleryOpenDownloads = function() {
@@ skipping 307 matching lines @@
 /**
  * Tests copy from drive's offline to drive's root.
  */
 testcase.transferFromOfflineToDrive = function() {
   testcase.intermediate.copyBetweenVolumes('Test Document.gdoc',
                                            'drive_offline',
                                            EXPECTED_FILES_IN_OFFLINE,
                                            'drive',
                                            EXPECTED_FILES_BEFORE_DRIVE);
 };