Files.app: Reland r261616: Add a test to rename a file.

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

Index: chrome/test/data/extensions/api_test/file_manager_browsertest/keyboard_operations.js
diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/keyboard_operations.js b/chrome/test/data/extensions/api_test/file_manager_browsertest/keyboard_operations.js
index 8709bec3bdfd3ea46442db673fa89ecbb2ff0de6..2dc192c0398b6c28bf9095ed2530539c08c5679b 100644
--- a/chrome/test/data/extensions/api_test/file_manager_browsertest/keyboard_operations.js
+++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/keyboard_operations.js
@@ -135,7 +135,6 @@ function testPromise(promise) {
   promise.then(function() {
     return new Promise(checkIfNoErrorsOccured);
   }).then(chrome.test.callbackPass(function() {

[mtomasz, 2014/04/09 03:38:54]

Why this change?

[hirono, 2014/04/09 20:01:13]

Because chrome.test.succeed generates a log.
We already have succeed log end of tests. So it generates succeed log doubly.

[mtomasz, 2014/04/09 21:15:19]

Please add a comment exactly saying that. Also, please comment that the
callbacPass is necessary to avoid prematurely finishing tests. Currently it
looks very misterious and is difficult to understand.

[hirono, 2014/04/09 21:37:06]

Done.

-    chrome.test.succeed();
   }), function(error) {
     chrome.test.fail(error.stack || error);
   });
@@ -183,6 +182,77 @@ function createNewFolder(path, initialEntrySet) {
   });
 };
 
+/**
+ * Renames a file.
+ * @param {string} windowId ID of window.

[mtomasz, 2014/04/09 03:38:54]

nit: window -> the window

[hirono, 2014/04/09 20:01:13]

Done.

+ * @param {string} oldName Old name of a file.
+ * @param {string} newName New name of a file.
+ * @return {Promise} Promise to be fulfilled on success.
+ */
+function renameFile(windowId, oldName, newName) {
+  return callRemoteTestUtil('selectFile', windowId, [oldName]).then(function() {
+    // Push Ctrl+Enter.
+    return fakeKeyDown(windowId, '#detail-table', 'Enter', true);
+  }).then(function() {
+    // Wait for rename text field.
+    return waitForElement(windowId, 'input.rename');
+  }).then(function() {
+    // Type new file name.
+    return callRemoteTestUtil(
+        'inputText', windowId, ['input.rename', newName]);
+  }).then(function() {
+    // Push Enter.
+    return fakeKeyDown(windowId, 'input.rename', 'Enter', false);
+  }).then(function() {
+    // Wait until rename completes.

[mtomasz, 2014/04/09 03:38:54]

I think this is racy. The class 'provisional' may not be added before
waitForElementLost is called.

[mtomasz, 2014/04/09 03:42:15]

To fix this race, we could eg. wait for the newName, and then for 'provisional'
class removed. Then we would have a guarantee that 'provisional' was set, and
later removed (since it is set together with the new name). WDYT?

[hirono, 2014/04/09 20:01:13]

This is race. But we cannot fix it by waiting newName.

1. Rename to newName.
2. It is an invalid name and the name is reverted.
3. Waiting newName. But it will not be seen.

I updated the lifetime of the marker so that we can ensure that renaming
completes.

[mtomasz, 2014/04/09 21:15:19]

Yes, that would cause a test timeout, so the test would be slower in case on
failures.

+    return waitForElementLost(windowId, '#detail-table .provisional');
+  });
+}
+
+/**
+ * Test for renaming a file.
+ * @param {string} path Initial path.
+ * @param {Array.<TestEntryInfo>} initialEntrySet Initial set of entries.
+ * @return {Promise} Promise to be fulfilled on success.
+ */
+function testRenameFile(path, initialEntrySet) {
+  var windowId;
+
+  // Make expected rows.
+  var initialExpectedEntryRows = TestEntryInfo.getExpectedRows(initialEntrySet);
+  var expectedEntryRows = TestEntryInfo.getExpectedRows(initialEntrySet);
+  for (var i = 0; i < expectedEntryRows.length; i++) {
+    if (expectedEntryRows[i][0] === 'hello.txt')
+      break;
+  }
+  expectedEntryRows[i][0] = 'New File Name.txt';

[mtomasz, 2014/04/09 03:38:54]

This works, but it is not readable. Can we move it to #225, under the if? We
could read it then:

'For every item in expected entry rows, if the name is hello.txt, then change it
to 'new file name.txt' and stop.'

[hirono, 2014/04/09 20:01:13]

Done.

+
+  // Open window.

[mtomasz, 2014/04/09 03:38:54]

nit: window -> a window

[hirono, 2014/04/09 20:01:13]

Done.

+  return new Promise(function(callback) {
+    setupAndWaitUntilReady(null, path, callback);
+  }).then(function(inWindowId) {
+    windowId = inWindowId;
+    return waitForFiles(windowId, initialExpectedEntryRows);
+  }).then(function(){
+    return renameFile(windowId, 'hello.txt', 'New File Name.txt');
+  }).then(function() {
+    // Wait for the new file name.
+    return waitForFiles(windowId,
+                        expectedEntryRows,
+                        {ignoreLastModifiedTime: true});
+  }).then(function() {
+    return renameFile(windowId, 'New File Name.txt', '.hidden file');
+  }).then(function() {
+    // The error dialog is shown.
+    return waitAndAcceptDialog(windowId);
+  }).then(function() {
+    // The name does not changed.

[mtomasz, 2014/04/09 03:38:54]

does not changed -> did not change OR has not changed OR is not changed

[hirono, 2014/04/09 20:01:13]

Done.

+    return waitForFiles(windowId,
+                        expectedEntryRows,
+                        {ignoreLastModifiedTime: true});
+  });
+};
+
 testcase.keyboardCopyDownloads = function() {
   keyboardCopy(RootPath.DOWNLOADS);
 };
@@ -206,3 +276,11 @@ testcase.createNewFolderDownloads = function() {
 testcase.createNewFolderDrive = function() {
   testPromise(createNewFolder(RootPath.DRIVE, BASIC_DRIVE_ENTRY_SET));
 };
+
+testcase.renameFileDownloads = function() {
+  testPromise(testRenameFile(RootPath.DOWNLOADS, BASIC_LOCAL_ENTRY_SET));
+};
+
+testcase.renameFileDrive = function() {
+  testPromise(testRenameFile(RootPath.DRIVE, BASIC_DRIVE_ENTRY_SET));
+};