Remove fake Bluetooth adapter from canonicalizeFilter tests

Remove fake Bluetooth adapter from canonicalizeFilter tests. setBlueToothFakeAdapter is causing these tests to fail when --batch-size=1 and is not necessary for them to pass. (see bug for more context)

BUG=650906
Committed: https://crrev.com/ad6d40ff799a200267e7cb25219970533199fc3e
Cr-Commit-Position: refs/heads/master@{#427405}

[jeffcarp, 2016-10-07 23:45:35]

Description was changed from

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests

BUG=650906
==========

to

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests

Still fails for
bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-optionalServices-member.html
... investigating

BUG=650906
==========

[jeffcarp, 2016-10-14 19:47:09]

Description was changed from

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests

Still fails for
bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-optionalServices-member.html
... investigating

BUG=650906
==========

to

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests.
setBlueToothFakeAdapter is causing these tests to fail when --batch-size=1 and
is not necessary for them to pass. (see bug for more context)

BUG=650906
==========

[jeffcarp, 2016-10-14 19:47:09]

jeffcarp@chromium.org changed reviewers:
+ ortuno@chromium.org, qyearsley@chromium.org

[qyearsley, 2016-10-14 20:39:28]

LGTM (although I didn't look very closely)!

[ortuno, 2016-10-17 03:21:32]

Promises and arrow functions are tricky :)

For a chain of promises to work you should always return a promise otherwise the
test will incorrectly succeed before the test code actually runs.

For example, similar to what we have in this CL:

promise_test(() => {
  (new Promise(resolve => setTimeout(resolve, 0)))
    .then(() => console.log('test'));
}).then(() => console.log('done'));

Prints:

'done'
'test'

But we want the following:

promise_test(() => {
  return (new Promise(resolve => setTimeout(resolve, 0)))
    .then(() => console.log('test'));
}).then(() => console.log('done'));

which will print 

'test'
'done'

In case you are wondering why there is currently no 'return' in the code it's
because if you use an expression right after an arrow function the return is
implicit:

// ...
  .then(() => 'yay');

is equivalent to:
// ...
  .then(() => { return 'yay' });

More about promises:
https://developers.google.com/web/fundamentals/getting-started/primers/promises

More about arrow functions:
https://googlechrome.github.io/samples/arrows-es6/

https://codereview.chromium.org/2408533002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):

https://codereview.chromium.org/2408533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:7:
promise_test(t => {
This should be:

promise_test(t => {
  return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
}, 'A filter must restrict the devices in some way.');

[jeffcarp, 2016-10-17 22:05:54]

On 2016/10/17 at 03:21:32, ortuno wrote:
> Promises and arrow functions are tricky :)
> 
> For a chain of promises to work you should always return a promise otherwise
the test will incorrectly succeed before the test code actually runs.
> 
> For example, similar to what we have in this CL:
> 
> promise_test(() => {
>   (new Promise(resolve => setTimeout(resolve, 0)))
>     .then(() => console.log('test'));
> }).then(() => console.log('done'));
> 
> Prints:
> 
> 'done'
> 'test'
> 
> But we want the following:
> 
> promise_test(() => {
>   return (new Promise(resolve => setTimeout(resolve, 0)))
>     .then(() => console.log('test'));
> }).then(() => console.log('done'));
> 
> which will print 
> 
> 'test'
> 'done'
> 
> In case you are wondering why there is currently no 'return' in the code it's
because if you use an expression right after an arrow function the return is
implicit:
> 
> // ...
>   .then(() => 'yay');
> 
> is equivalent to:
> // ...
>   .then(() => { return 'yay' });
> 
> More about promises:
>
https://developers.google.com/web/fundamentals/getting-started/primers/promises
> 
> More about arrow functions:
> https://googlechrome.github.io/samples/arrows-es6/
> 
>
https://codereview.chromium.org/2408533002/diff/20001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):
> 
>
https://codereview.chromium.org/2408533002/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:7:
promise_test(t => {
> This should be:
> 
> promise_test(t => {
>   return promise_rejects(t, new TypeError(),
requestDeviceWithKeyDown({filters: [{}]}));
> }, 'A filter must restrict the devices in some way.');

Thanks for catching that, updated.

[ortuno, 2016-10-18 00:25:27]

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

[commit-bot: I haz the power, 2016-10-18 00:26:07]

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

[ortuno, 2016-10-18 00:33:33]

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:8:
return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
nit: only two spaces for indentation. Please fix all files.

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-services-member.html
(right):

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-services-member.html:9:
test_promises = test_promises.then(() => promise_rejects(t, TypeError(),
requestDeviceWithKeyDown(args)));
nit: Please keep lines shorter than 100 characters.

[commit-bot: I haz the power, 2016-10-18 01:14:50]

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

[commit-bot: I haz the power, 2016-10-18 01:14:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jeffcarp, 2016-10-18 16:04:21]

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):

https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:8:
return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
On 2016/10/18 at 00:33:32, ortuno wrote:
> nit: only two spaces for indentation. Please fix all files.

Is there a standard or a linter for this? I ask because the WPT are indented 4
spaces. I'm fine with either, I just want all layout tests to be consistent.

[qyearsley, 2016-10-18 16:44:41]

On 2016/10/18 at 16:04:21, jeffcarp wrote:
>
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):
> 
>
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:8:
return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
> On 2016/10/18 at 00:33:32, ortuno wrote:
> > nit: only two spaces for indentation. Please fix all files.
> 
> Is there a standard or a linter for this? I ask because the WPT are indented 4
spaces. I'm fine with either, I just want all layout tests to be consistent.

The style guide
(https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-...)
says that either two or four spaces is fine, as long as you're consistent within
a file.

Some layout tests use four-space indentation, some use two; between different
layout tests, style is not consistent, and we're not running any linters. WPT
does have a linter, but I don't think it checks for two or four space
indentation; it just checks that you're not using tabs.

[jeffcarp, 2016-10-18 16:55:33]

On 2016/10/18 at 16:44:41, qyearsley wrote:
> On 2016/10/18 at 16:04:21, jeffcarp wrote:
> >
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
> > File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):
> > 
> >
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
> >
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:8:
return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
> > On 2016/10/18 at 00:33:32, ortuno wrote:
> > > nit: only two spaces for indentation. Please fix all files.
> > 
> > Is there a standard or a linter for this? I ask because the WPT are indented
4 spaces. I'm fine with either, I just want all layout tests to be consistent.
> 
> The style guide
(https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-...)
says that either two or four spaces is fine, as long as you're consistent within
a file.
> 
> Some layout tests use four-space indentation, some use two; between different
layout tests, style is not consistent, and we're not running any linters. WPT
does have a linter, but I don't think it checks for two or four space
indentation; it just checks that you're not using tabs.

Ok thanks! I'll revert all these files to 2-space indentation.

[jeffcarp, 2016-10-25 00:20:11]

On 2016/10/18 at 16:55:33, jeffcarp wrote:
> On 2016/10/18 at 16:44:41, qyearsley wrote:
> > On 2016/10/18 at 16:04:21, jeffcarp wrote:
> > >
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
> > > File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html
(right):
> > > 
> > >
https://codereview.chromium.org/2408533002/diff/40001/third_party/WebKit/Layo...
> > >
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/empty-filter.html:8:
return promise_rejects(t, new TypeError(), requestDeviceWithKeyDown({filters:
[{}]}));
> > > On 2016/10/18 at 00:33:32, ortuno wrote:
> > > > nit: only two spaces for indentation. Please fix all files.
> > > 
> > > Is there a standard or a linter for this? I ask because the WPT are
indented 4 spaces. I'm fine with either, I just want all layout tests to be
consistent.
> > 
> > The style guide
(https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-...)
says that either two or four spaces is fine, as long as you're consistent within
a file.
> > 
> > Some layout tests use four-space indentation, some use two; between
different layout tests, style is not consistent, and we're not running any
linters. WPT does have a linter, but I don't think it checks for two or four
space indentation; it just checks that you're not using tabs.
> 
> Ok thanks! I'll revert all these files to 2-space indentation.

Ok, indentation is fixed. There were also two tests that weren't passing after I
fixed the promise issue - I think they still need the fake adapter.

the tests in question:
bluetooth/requestDevice/canonicalizeFilter/unicode-valid-length-name-name.html
bluetooth/requestDevice/canonicalizeFilter/unicode-valid-length-name-namePrefix.html

[ortuno, 2016-10-25 01:44:00]

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

[commit-bot: I haz the power, 2016-10-25 01:44:33]

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, 2016-10-25 04:22:29]

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

[commit-bot: I haz the power, 2016-10-25 04:22:30]

Dry run: This issue passed the CQ dry run.

[ortuno, 2016-10-25 04:38:15]

lgtm bar one test fix.

https://codereview.chromium.org/2408533002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-services-member.html
(right):

https://codereview.chromium.org/2408533002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/wrong-service-in-services-member.html:9:
test_promises = test_promises.then(() => (
nit: There is one extra parenthesis.

test_promises = test_promises.then(() => promise_rejects(
  t, TypeError(), requestDeviceWithKeyDown(args)));

[jeffcarp, 2016-10-25 16:57:36]

The CQ bit was checked by jeffcarp@chromium.org

[jeffcarp, 2016-10-25 16:57:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
qyearsley@chromium.org, ortuno@chromium.org
Link to the patchset: https://codereview.chromium.org/2408533002/#ps80001
(title: "Fix two small indentation issues")

[commit-bot: I haz the power, 2016-10-25 16:58:11]

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, 2016-10-25 17:52:27]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-25 18:03:27]

Description was changed from

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests.
setBlueToothFakeAdapter is causing these tests to fail when --batch-size=1 and
is not necessary for them to pass. (see bug for more context)

BUG=650906
==========

to

==========
Remove fake Bluetooth adapter from canonicalizeFilter tests.
setBlueToothFakeAdapter is causing these tests to fail when --batch-size=1 and
is not necessary for them to pass. (see bug for more context)

BUG=650906

Committed: https://crrev.com/ad6d40ff799a200267e7cb25219970533199fc3e
Cr-Commit-Position: refs/heads/master@{#427405}
==========

[commit-bot: I haz the power, 2016-10-25 18:03:28]

Patchset 5 (id:??) landed as
https://crrev.com/ad6d40ff799a200267e7cb25219970533199fc3e
Cr-Commit-Position: refs/heads/master@{#427405}