Fix promises in file manager device handler unit tests.

Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Switch to creating promises inline to make this less likely
in the future.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2650403005
Cr-Commit-Position: refs/heads/master@{#446547}
Committed: https://chromium.googlesource.com/chromium/src/+/78326029c3f06738fd2169474282c2d354d5ea28

[Daniel Erat, 2017-01-26 19:50:29]

Description was changed from

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Also fix indenting in a few places.

BUG=none
==========

to

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Also fix indenting in a few places.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Daniel Erat, 2017-01-26 19:52:17]

derat@chromium.org changed reviewers:
+ fukino@chromium.org

[Daniel Erat, 2017-01-26 19:52:18]

i think that this fixes the problem i encountered in
https://codereview.chromium.org/2645353005/. please let me know if you have
suggestions for making the CL description clearer -- i'm still a beginner when
it comes to JS promises. :-)

[Daniel Erat, 2017-01-26 19:52:23]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 19:53:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 20:21:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 20:21:35]

Dry run: This issue passed the CQ dry run.

[fukino, 2017-01-26 22:33:24]

Thank you for the investigation and fixing the issue!
lgtm assuming a comment is resolved.

https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/background/js/device_handler_unittest.js
(right):

https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/background/js/device_handler_unittest.js:153:
reportPromise(resolver.promise, callback);
resolver.promise should be promise here, too.

[Daniel Erat, 2017-01-26 22:33:28]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 22:34:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Daniel Erat, 2017-01-26 22:37:53]

https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/background/js/device_handler_unittest.js
(right):

https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/background/js/device_handler_unittest.js:153:
reportPromise(resolver.promise, callback);
On 2017/01/26 22:33:24, fukino wrote:
> resolver.promise should be promise here, too.

whoops. thanks for catching this!

let me know if you think it'd be cleaner/safer to inline the .then calls that
create the promise within the reportPromise calls, e.g.

  reportPromise(
      resolver.promise.then(
          function(event) {
            assertEquals('blabbity', event.volumeId);
          }),
      callback);

[Daniel Erat, 2017-01-26 22:37:59]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 22:38:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fukino, 2017-01-26 22:42:26]

On 2017/01/26 22:37:53, Daniel Erat wrote:
>
https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
> File ui/file_manager/file_manager/background/js/device_handler_unittest.js
> (right):
> 
>
https://codereview.chromium.org/2650403005/diff/1/ui/file_manager/file_manage...
> ui/file_manager/file_manager/background/js/device_handler_unittest.js:153:
> reportPromise(resolver.promise, callback);
> On 2017/01/26 22:33:24, fukino wrote:
> > resolver.promise should be promise here, too.
> 
> whoops. thanks for catching this!
> 
> let me know if you think it'd be cleaner/safer to inline the .then calls that
> create the promise within the reportPromise calls, e.g.
> 
>   reportPromise(
>       resolver.promise.then(
>           function(event) {
>             assertEquals('blabbity', event.volumeId);
>           }),
>       callback);

Both are OK, but the style you suggested looks a bit clearer to me.

[commit-bot: I haz the power, 2017-01-26 23:14:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 23:14:43]

Dry run: This issue passed the CQ dry run.

[Daniel Erat, 2017-01-27 00:03:59]

Description was changed from

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Also fix indenting in a few places.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Switch to creating promises inline to make this less likely
in the future.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Daniel Erat, 2017-01-27 00:04:08]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[Daniel Erat, 2017-01-27 00:05:04]

mind taking another look? i've moved all of the promise creation inline to
hopefully make this less likely in the future. (i still feel like i don't fully
understand javascript indenting; hopefully i didn't get it too wrong.)

[commit-bot: I haz the power, 2017-01-27 00:08:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fukino, 2017-01-27 00:09:19]

LGTM.
Indentation looks correct.
Thank you for making them clearer & safer!

[Daniel Erat, 2017-01-27 00:20:07]

The CQ bit was unchecked by derat@chromium.org

[Daniel Erat, 2017-01-27 00:20:11]

The CQ bit was checked by derat@chromium.org

[commit-bot: I haz the power, 2017-01-27 00:20:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 02:39:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485476411905020,
"parent_rev": "11048b2f4c969c558eb594f4500b14d4c250d9e4", "commit_rev":
"78326029c3f06738fd2169474282c2d354d5ea28"}

[commit-bot: I haz the power, 2017-01-27 02:39:49]

Description was changed from

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Switch to creating promises inline to make this less likely
in the future.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix promises in file manager device handler unit tests.

Several tests for chrome.fileManagerPrivate's DeviceHandler
class were failing to pass the correct promises to
reportPromise(), resulting in failed assertions being
ignored.

Switch to creating promises inline to make this less likely
in the future.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2650403005
Cr-Commit-Position: refs/heads/master@{#446547}
Committed:
https://chromium.googlesource.com/chromium/src/+/78326029c3f06738fd2169474282...
==========

[commit-bot: I haz the power, 2017-01-27 02:39:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/78326029c3f06738fd2169474282...