Files.app: Add a test to open a file dialog box.

Files.app: Add a test to open a file dialog box.

BUG=345059
TEST=run the test

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277727

[hirono, 2014-06-16 10:21:42]

PTAL the CL? Thank you!

[fukino, 2014-06-17 05:59:10]

Sorry to be late!

https://codereview.chromium.org/331583003/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/file_manager/file_manager_browsertest.cc (right):

https://codereview.chromium.org/331583003/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/file_manager/file_manager_browsertest.cc:963:
MAYBE_ExecuteDefaultTaskOnDrive,
Wow! Thank you for catching this.

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js
(right):

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:19:
var resultProimse = new Promise(function(fulfill) {
nit: s/resultProimse/resultPromise/

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:27:
return callRemoteTestUtil('selectVolume', id, [volumeName]);
nit: I think this indent level should be 6 spaces and following lines should be
4 spaces shallower.

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:31:
return waitForFiles(id, expectedRows);
In my workistation, OpenFileDialog/FileManagerBrowserTest.Test/2 timed out 7
times in 100 runs by stacking in this waitForFiles. Could you take a look?

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:45:
});
nit: indent level should be same as previous then.

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/background.js (right):

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/background.js:552: * @type {string}
nit: s/string/RegExp/ ?

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/test_util.js (right):

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/test_util.js:59:
test.util.sync.getWindows = function(callback) {
nit: |callback| unused

[hirono, 2014-06-17 07:07:14]

Thank you!

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js
(right):

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:19:
var resultProimse = new Promise(function(fulfill) {
On 2014/06/17 05:59:10, fukino wrote:
> nit: s/resultProimse/resultPromise/

Done.

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:27:
return callRemoteTestUtil('selectVolume', id, [volumeName]);
On 2014/06/17 05:59:10, fukino wrote:
> nit: I think this indent level should be 6 spaces and following lines should
be
> 4 spaces shallower.

When I started to use Promise, I and @mtomasz discussed about the style of
Promise briefly but didn't conclude.
Let's discuss about the style again.

https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:31:
return waitForFiles(id, expectedRows);
On 2014/06/17 05:59:10, fukino wrote:
> In my workistation, OpenFileDialog/FileManagerBrowserTest.Test/2 timed out 7
> times in 100 runs by stacking in this waitForFiles. Could you take a look?

Good catch! It looks the test clicks the navigation list before it is
initialized.
I fixed the code and ran the test 100 times. It passed.

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/background.js (right):

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/background.js:552: * @type {string}
On 2014/06/17 05:59:10, fukino wrote:
> nit: s/string/RegExp/ ?

Done.

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
File ui/file_manager/file_manager/background/js/test_util.js (right):

https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
ui/file_manager/file_manager/background/js/test_util.js:59:
test.util.sync.getWindows = function(callback) {
On 2014/06/17 05:59:10, fukino wrote:
> nit: |callback| unused

Done.

[fukino, 2014-06-17 07:23:49]

On 2014/06/17 07:07:14, hirono wrote:
> Thank you!
> 
>
https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
> File
>
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js
> (right):
> 
>
https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
>
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:19:
> var resultProimse = new Promise(function(fulfill) {
> On 2014/06/17 05:59:10, fukino wrote:
> > nit: s/resultProimse/resultPromise/
> 
> Done.
> 
>
https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
>
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:27:
> return callRemoteTestUtil('selectVolume', id, [volumeName]);
> On 2014/06/17 05:59:10, fukino wrote:
> > nit: I think this indent level should be 6 spaces and following lines should
> be
> > 4 spaces shallower.
> 
> When I started to use Promise, I and @mtomasz discussed about the style of
> Promise briefly but didn't conclude.
> Let's discuss about the style again.
> 
>
https://codereview.chromium.org/331583003/diff/60001/chrome/test/data/extensi...
>
chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/file_dialog.js:31:
> return waitForFiles(id, expectedRows);
> On 2014/06/17 05:59:10, fukino wrote:
> > In my workistation, OpenFileDialog/FileManagerBrowserTest.Test/2 timed out 7
> > times in 100 runs by stacking in this waitForFiles. Could you take a look?
> 
> Good catch! It looks the test clicks the navigation list before it is
> initialized.
> I fixed the code and ran the test 100 times. It passed.
> 
>
https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
> File ui/file_manager/file_manager/background/js/background.js (right):
> 
>
https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
> ui/file_manager/file_manager/background/js/background.js:552: * @type {string}
> On 2014/06/17 05:59:10, fukino wrote:
> > nit: s/string/RegExp/ ?
> 
> Done.
> 
>
https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
> File ui/file_manager/file_manager/background/js/test_util.js (right):
> 
>
https://codereview.chromium.org/331583003/diff/60001/ui/file_manager/file_man...
> ui/file_manager/file_manager/background/js/test_util.js:59:
> test.util.sync.getWindows = function(callback) {
> On 2014/06/17 05:59:10, fukino wrote:
> > nit: |callback| unused
> 
> Done.

lgtm. Thanks!

[hirono, 2014-06-17 07:37:32]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-06-17 07:40:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hirono@chromium.org/331583003/100001

[commit-bot: I haz the power, 2014-06-17 11:29:46]

Change committed as 277727