860697 - QEMU Bluetooth tests: local adapter features

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[John Lin [:jhlin][:jolin]]

Implement Marionette tests for Gecko Bluetooth API to be run on B2G emulator:
- turn on/off Bluetooth
- get default adapter
- get adapter properties: BD address, name, discoverable
- set mutable adapter properties: name, discoverable

Comment 1

[John Lin [:jhlin][:jolin]]

test cases implementation: https://github.com/jhlin/mozilla-central/commit/dfa7d6996db45060367f3771d052c0297f426e8d

Comment 2

[John Lin [:jhlin][:jolin]]

Please note that to pass the tests, attachment 743499 [details] [diff] [review] is needed.

Comment 3

[u459114]

to reflect the truth that John is not working on BT anymore. Macro, please check who should own this issue

Comment 4

[Eric Chou [:ericchou] [:echou]]

Jamin will take over John's work.

Comment 5

[Eric Chou [:ericchou] [:echou]]

(In reply to John Lin[:jolin][:jhlin] from comment #0)
> Implement Marionette tests for Gecko Bluetooth API to be run on B2G emulator:
> - turn on/off Bluetooth
> - get default adapter

The above two items have already been covered after bug 939637 and bug 939441 landed. So Jamin will focus on other properties just like what John listed below.

> - get adapter properties: BD address, name, discoverable
> - set mutable adapter properties: name, discoverable

Comment 6

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v1)

Comment 7

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8364219 [details] [diff] [review]
[Mnw] Add test cases to set/get  properties of BluetoothAdapter. (v1)

Review of attachment 8364219 [details] [diff] [review]:
-----------------------------------------------------------------

Overall looks good. Please fix nits I mentioned. Thanks.

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_getters.js
@@ +29,5 @@
> +  ADDRESS : 1,
> +  COD : 2,
> +  DISCOVERABLE : 3,
> +  DISCOVERING : 4,
> +  DISCOVERABLETIMEOUT : 5

How about 'DISCOVERABLE_TIMEOUT'? (Just like MARIONETTE_TIMEOUT above)

@@ +88,5 @@
> +  testAdapterGetter(aAdapter, PropertyEnum.COD);
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLE);
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERING);
> +  /* testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLETIMEOUT); */
> +  // We didn't check discoverableTimeout here since we can't directly set the

I'll handle this in my patch. Please re-add this check.

@@ +89,5 @@
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLE);
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERING);
> +  /* testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLETIMEOUT); */
> +  // We didn't check discoverableTimeout here since we can't directly set the
> +  // value through emulatoer command.

typo: emulator

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
@@ +24,5 @@
> +function setName(aAdapter, aName) {
> +  let deferred = Promise.defer();
> +
> +  let request = aAdapter.setName(aName);
> +  request.addEventListener("success", function() {

You can hook the callback function by doing 'request.onsuccess = function () { ..... }'. Here and below.

@@ +75,5 @@
> +startBluetoothTest(true, function testCaseMain(aAdapter) {
> +  log("Testing BluetoothAdapter setters ...");
> +
> +  return setDiscoverable(aAdapter, true)
> +    .then(function() { return setDiscoverableTimeout(aAdapter,180); })

super-nit: need to insert a blank after before '180'

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8364219 [details] [diff] [review]
[Mnw] Add test cases to set/get  properties of BluetoothAdapter. (v1)

Review of attachment 8364219 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/head.js
@@ +33,5 @@
> +
> +  /**
> +   * @return Promise
> +   */
> +  function waitFinish() {

Just drop by, but I'd say I really don't consider overriding `waitFinish` a good idea.

First, I don't know if there are still other clean-ups to do.  If every clean-up routine takes the same trick and overrides `waitFinish`, then you'll soon get yourself caught in a fire.  No! Don't do that.  There is already a `cleanUp` function in http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/tests/marionette/head.js#231 .

Second, hide underlying clean-ups, setup routines as possible.  It is the reason we have a "head.js".  If we hide |runEmulatorCmd| for pending command counts guard, then we should probably also hide routines to wait for such counter.  Don't just create another MUST-DO thing.

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_getters.js
@@ +34,5 @@
> +};
> +
> +function testAdapterGetter(aAdapter, aBtAdapterProperty) {
> +  switch(aBtAdapterProperty) {
> +    case PropertyEnum.NAME:

That's really not of Promise style.

@@ +35,5 @@
> +
> +function testAdapterGetter(aAdapter, aBtAdapterProperty) {
> +  switch(aBtAdapterProperty) {
> +    case PropertyEnum.NAME:
> +      emulator.run("bt get local name", function(result) {

`get` is redundant here.  You can have `bt local name` as getter and `bt local name <new_name>` as setter.

@@ +39,5 @@
> +      emulator.run("bt get local name", function(result) {
> +        let name = result[0];
> +        log("  Get adapter name: " + name);
> +        is(result[1], "OK", "QEMU command [bt get local name].");
> +        is(name, aAdapter.name, "BluetoothAdapter.name");

Looks like we can have one common function that tests all attributes:

  function testAdapterGetter(aAdapter, aPropName, aParamName) {
    let cmd = "bt local " + aParamName;
    return runEmulatorCmd(cmd)
      .then(function(aResult) {
        let value = result[0];
        is(result[1], "OK", "QEMU command [bt get local name].");
        is(value, aAdapter[aPropName], "BluetoothAdapter." + aPropName);
      });
  }

@@ +87,5 @@
> +  testAdapterGetter(aAdapter, PropertyEnum.ADDRESS);
> +  testAdapterGetter(aAdapter, PropertyEnum.COD);
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLE);
> +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERING);
> +  /* testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLETIMEOUT); */

Then we do:

  return testAdapterGetter(aAdapter, "name", "name")
    .then(() => testAdapterGetter(aAdapter, "address", "address"))
    .then(() => testAdapterGetter(aAdapter, "class", "cod"))
    ...

Comment 9

[Eric Chou [:ericchou] [:echou]]

> @@ +88,5 @@
> > +  testAdapterGetter(aAdapter, PropertyEnum.COD);
> > +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLE);
> > +  testAdapterGetter(aAdapter, PropertyEnum.DISCOVERING);
> > +  /* testAdapterGetter(aAdapter, PropertyEnum.DISCOVERABLETIMEOUT); */
> > +  // We didn't check discoverableTimeout here since we can't directly set the
> 
> I'll handle this in my patch. Please re-add this check.

I just found that discoverableTimeout is a helper which is provided by BlueZ to turn off inquiry scan automatically. There's no mapping HCI commands so I would say we shouldn't test this case.

Comment 10

[Jamin Liu [:jaliu][:Jamin]]

Hi Vicamo,
Thank you for you helpful comments.
I will create a new patch based on your suggestions.

Since the emulator commands of BT would be modified as well, my patch would be upload after the emulator commands are finalized in Bug 860697.
Regards.

Comment 11

[Jamin Liu [:jaliu][:Jamin]]

Hi Eric,
Got it.
The discoverable timeout are going to be removed from my next patch.
Thank you.

Comment 12

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v2)

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8373156 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter. (v2)

Review of attachment 8373156 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_getters.js
@@ +35,5 @@
> +function testAdapterGetter(aAdapter, aPropertyEnum) {
> +  let deferred = Promise.defer();
> +  switch(aPropertyEnum) {
> +    case PropertyEnum.NAME:
> +      runEmulatorCmd("bt property local name", function(result) {

1) We MUST wrap `runEmulatorCmd` to keep tracking of its execution count.  See [1] for an example.

2) You can always populate "head.js" with new convenient utility functions.  That's what it's meant for.

3) I'd like to extract common parts to an utility function.  So something like:

  function testAdapterGetter(aAdapter, aPropertyName, aParamName, aExpected) {
    let cmd = "bt property local " + aParamName;
    return runEmulatorCmdSafe(cmd)
      .then(function(aResults) {
        log("  Got adapter " + aPropertyName + ": " + aResults[0]);
        is(aResults[0], aExpected, "BluetoothAdapter." + aPropertyName);
      });
  }

  startBluetoothTest(true, function testCaseMain(aAdapter) {
    log("Checking the correctness of BluetoothAdapter properties ...");

    return        testAdapterGetter(aAdapter, "name",         "name",         aAdapter.name)
      .then(() => testAdapterGetter(aAdapter, "address",      "address",      aAdapter.address))
      .then(() => testAdapterGetter(aAdapter, "class",        "cod",          aAdapter.class.toString(16)))
      .then(() => testAdapterGetter(aAdapter, "discoverable", "discoverable", aAdapter.discoverable.toString()))
      .then(() => testAdapterGetter(aAdapter, "discovering",  "discovering",  aAdapter.discovering.toString()));
  }

[1]: https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/tests/marionette/head.js#272

@@ +89,5 @@
> +startBluetoothTest(true, function testCaseMain(aAdapter) {
> +  log("Checking the correctness of BluetoothAdapter properties ...");
> +
> +  return   testAdapterGetter( aAdapter, PropertyEnum.NAME )
> +    .then( testAdapterGetter( aAdapter, PropertyEnum.ADDRESS ))

This means two or more `testAdapterGetter` calls will be first executed almost concurrently, passing their results as arguments of `promise.then()`, which is not quite safe because it depends on the ability to multiplex emulator commands in function `runEmulatorCmd`.

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
@@ +20,5 @@
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +const BT_ADAPTER_NAME = "Some user friendly name for BT device";
> +
> +function setName(aAdapter, aName) {

Again, you can always populate "head.js" with new convenient utility functions if you feel appropriate.

@@ +26,5 @@
> +
> +  let request = aAdapter.setName(aName);
> +  request.onsuccess = function () {
> +    log("  setName succeed.");
> +    is(aAdapter.name, BT_ADAPTER_NAME, "Failed to set name.");

"aAdapter.name".  The third parameter of `is()` is the name/summary of this test, not passString or failString.  See https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-simpletest.js#40

@@ +76,5 @@
> +  log("Testing BluetoothAdapter setters ...");
> +
> +  return setDiscoverable(aAdapter, true)
> +    .then( setDiscoverableTimeout(aAdapter, 180) )
> +    .then( setName(aAdapter, BT_ADAPTER_NAME) );

Maybe you can consider reverting these states and test again.  Two benefits here: 1) that covers more possible values of that API, 2) reverting changes made in this test script prevents execution ordering problem in further test scripts.  For example, bug 970804 is to enable OOP mode of Marionette tests.  One test script will be executed twice.  Without setting device name back, the second run of "test_dom_BluetoothAdapter_getters.js" will undoubtedly fail.

Comment 14

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v3)

Comment 15

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8375443 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter. (v3)

Review of attachment 8375443 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/head.js
@@ +30,5 @@
> +const EMULATOR_CLASS = 0x58020c;
> +
> +const REMOTE_DEVICE_ADDRESS = "11:22:33:44:55:66";
> +
> +const REMOTE_DEVICE_NAME = "User_friendly_name_for_remote_device";

There could be multiple remote devices.  I wouldn't do this if I were you.  But that's more like personal taste, it's fine here.

@@ +33,5 @@
> +
> +const REMOTE_DEVICE_NAME = "User_friendly_name_for_remote_device";
> +
> +// Enums of BT adapter property
> +let PropertyEnum = {

You don't really need enums.  I've give you an example in previous review.

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_getters.js
@@ +23,5 @@
> +MARIONETTE_TIMEOUT = 60000;
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +function testAdapterGetter(aAdapter, aPropertyEnum) {
> +  switch(aPropertyEnum) {

I really don't like enums.  Using enums reveals that one doesn't really look into the code and dig out the common parts.  Sorry.

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
@@ +18,5 @@
> +
> +MARIONETTE_TIMEOUT = 60000;
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +const BT_DEVICE_NAME = "User_friendly_name_for_BT_device";

REMOTE_DEVICE_NAME

@@ +77,5 @@
> +
> +  return Promise.resolve()
> +    .then( () => setDiscoverable(aAdapter, true) )
> +    .then( () => setDiscoverableTimeout(aAdapter, 180) )
> +    .then( () => setName(aAdapter, BT_DEVICE_NAME) )

Shouldn't the order be setName() => setDiscoverableTimeout => setDiscoverable()?

s/BT_DEVICE_NAME/REMOTE_DEVICE_NAME/

Comment 16

[Jamin Liu [:jaliu][:Jamin]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #13)
> Comment on attachment 8373156 [details] [diff] [review]
> [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v2)
>   function testAdapterGetter(aAdapter, aPropertyName, aParamName, aExpected)
> {
>     let cmd = "bt property local " + aParamName;
>     return runEmulatorCmdSafe(cmd)
>       .then(function(aResults) {
>         log("  Got adapter " + aPropertyName + ": " + aResults[0]);
>         is(aResults[0], aExpected, "BluetoothAdapter." + aPropertyName);
>       });
>   }
> 
>   startBluetoothTest(true, function testCaseMain(aAdapter) {
>     log("Checking the correctness of BluetoothAdapter properties ...");
> 
>     return        testAdapterGetter(aAdapter, "name",         "name",       
> aAdapter.name)
>       .then(() => testAdapterGetter(aAdapter, "address",      "address",    
> aAdapter.address))
>       .then(() => testAdapterGetter(aAdapter, "class",        "cod",        
> aAdapter.class.toString(16)))
>       .then(() => testAdapterGetter(aAdapter, "discoverable",
> "discoverable", aAdapter.discoverable.toString()))
>       .then(() => testAdapterGetter(aAdapter, "discovering",  "discovering",
> aAdapter.discovering.toString()));
>   }

I think this is a great idea and would make code cleaner.
However, I still use switch case to handle the QEMU command here, since I want to make the function testAdapterGetter() is easier to use. I concern about some of the user don't familiar with QEMU BT command, so I try to avoid to put BT command as parameters of function.

I'm glad to refine the patch if you have any suggestion.
Regards

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Jamin Liu [:jaliu] from comment #16)
> However, I still use switch case to handle the QEMU command here, since I
> want to make the function testAdapterGetter() is easier to use. I concern
> about some of the user don't familiar with QEMU BT command, so I try to
> avoid to put BT command as parameters of function.

For someone not familiar enough with what he's going to do, either he should not take/be assigned with the job at the first place, or the reviewers should guide him through the process.  I don't know why there's an argument about this.

> I'm glad to refine the patch if you have any suggestion.

I've made my review about this, and you still own the choice not marking me as the reviewer.

Comment 18

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v4)

Comment 19

[Jamin Liu [:jaliu][:Jamin]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #17)
> (In reply to Jamin Liu [:jaliu] from comment #16)
> > However, I still use switch case to handle the QEMU command here, since I
> > want to make the function testAdapterGetter() is easier to use. I concern
> > about some of the user don't familiar with QEMU BT command, so I try to
> > avoid to put BT command as parameters of function.
> 
> For someone not familiar enough with what he's going to do, either he should
> not take/be assigned with the job at the first place, or the reviewers
> should guide him through the process.  I don't know why there's an argument
> about this.
> 
> > I'm glad to refine the patch if you have any suggestion.
> 
> I've made my review about this, and you still own the choice not marking me
> as the reviewer.

Hi Vicamo,
Got it. I uploaded a new patch. 
Thank you for your time and comment.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8376153 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter. (v4)

Review of attachment 8376153 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
@@ +80,5 @@
> +    .then( () => setDiscoverable(aAdapter, true) )
> +    .then( () => setDiscoverableTimeout(aAdapter, 180) )
> +    .then( () => setName(aAdapter, EMULATOR_NAME) )
> +    .then( () => setDiscoverable(aAdapter, false) )
> +    .then( () => setDiscoverableTimeout(aAdapter, 120) );

I still feel the order here is very strange.  When we called |setDiscoverable()|, what's the timeout used at the time?  This could be irrelevant for the test here, so I'm going to leave it for Eric.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8376153 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter. (v4)

Review of attachment 8376153 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_getters.js
@@ +23,5 @@
> +MARIONETTE_TIMEOUT = 60000;
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +function testAdapterGetter(aAdapter, aPropertyName, aParamName, aExpected) {
> +  let cmd = "bt property local " + aParamName;

I'm not going to have local/remote device properties concept in bug 860696.  You can just call `bt property <bd_addr>` to any available devices.  For self reference to that LOCAL dungle, use BDADDR_LOCAL (ff:ff:ff:00:00:00) instead.  See bug 860698 comment 14.

Comment 22

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v5)

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8398300 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter. (v5)

Review of attachment 8398300 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/tests/marionette/head.js
@@ +32,5 @@
> +// Use same definition in QEMU for special bluetooth address,
> +// which were defined at external/qemu/hw/bt.h:
> +const BDADDR_ANY	 = "00:00:00:00:00:00";
> +const BDADDR_ALL   = "ff:ff:ff:ff:ff:ff";
> +const BDADDR_LOCAL = "ff:ff:ff:00:00:00";

nit: align "BDADDR_ANY   ="

Comment 24

[Jamin Liu [:jaliu][:Jamin]]

Thank you. :)

I've fixed this alignment problem.
The patch is on the try server.

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #23)
> Comment on attachment 8398300 [details] [diff] [review]
> [Mnw] Add test cases to set/get properties of BluetoothAdapter. (v5)
> 
> Review of attachment 8398300 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bluetooth/tests/marionette/head.js
> @@ +32,5 @@
> > +// Use same definition in QEMU for special bluetooth address,
> > +// which were defined at external/qemu/hw/bt.h:
> > +const BDADDR_ANY	 = "00:00:00:00:00:00";
> > +const BDADDR_ALL   = "ff:ff:ff:ff:ff:ff";
> > +const BDADDR_LOCAL = "ff:ff:ff:00:00:00";
> 
> nit: align "BDADDR_ANY   ="

Comment 25

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. r=vyang

It looks fine on try server.
https://tbpl.mozilla.org/?tree=Try&rev=1d2f306cce0d

Comment 26

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8399207 [details] [diff] [review]
[Mnw] Add test cases to set/get properties of BluetoothAdapter.  r=vyang

Review of attachment 8399207 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Jamin, please see my comments below.

::: dom/bluetooth/tests/marionette/head.js
@@ +302,5 @@
> + * Wait for one named BluetoothAdapter event.
> + *
> + * Resolve if that named event occurs.  Never reject.
> + *
> + * Forfill params: the DOMEvent passed.

typo: fulfill

@@ +307,5 @@
> + *
> + * @param aAdapter
> + *        The BluetoothAdapter you want to use.
> + * @param aEventName
> + *        The the name of the EventHandler.

typo: an additional 'the'

::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
@@ +80,5 @@
> +    .then( () => setDiscoverableTimeout(aAdapter, 180) )
> +    .then( () => setDiscoverable(aAdapter, true) )
> +    .then( () => setName(aAdapter, EMULATOR_NAME) )
> +    .then( () => setDiscoverable(aAdapter, false) )
> +    .then( () => setDiscoverableTimeout(aAdapter, 120) );

Question: the order of calling setDiscoverable() and setDiscoverableTimeout() are upside down. Do you tend to do this?

Moreover, since we won't have setDiscoverableTimeout() API in new WebBluetooth API design, I would like to suggest that we should remove setting this property in our test case.

Comment 27

[Jamin Liu [:jaliu][:Jamin]]

Hi Eric,

Yes, I choose to set timeout before it become discoverable. I try to avoid changing the discoverable timeout when it's still discoverble, since we don't have a clear definition for this behavior.

The order I expected is:
[Set Timeout] -> [Start being discoverable]
[Stop Being discoverable] -> [Set Timeout]

Currently, setDiscoverableTimeout() is still a setter of Bluetooth adapter.
Thus, I think it's reasonable to add this API to this test to cover every setter APIs. 

(In reply to Eric Chou [:ericchou] [:echou] from comment #26)
> Comment on attachment 8399207 [details] [diff] [review]
> [Mnw] Add test cases to set/get properties of BluetoothAdapter.  r=vyang
> 
> Review of attachment 8399207 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Jamin, please see my comments below.
> 
> ::: dom/bluetooth/tests/marionette/head.js
> @@ +302,5 @@
> > + * Wait for one named BluetoothAdapter event.
> > + *
> > + * Resolve if that named event occurs.  Never reject.
> > + *
> > + * Forfill params: the DOMEvent passed.
> 
> typo: fulfill
> 
> @@ +307,5 @@
> > + *
> > + * @param aAdapter
> > + *        The BluetoothAdapter you want to use.
> > + * @param aEventName
> > + *        The the name of the EventHandler.
> 
> typo: an additional 'the'
> 
> ::: dom/bluetooth/tests/marionette/test_dom_BluetoothAdapter_setters.js
> @@ +80,5 @@
> > +    .then( () => setDiscoverableTimeout(aAdapter, 180) )
> > +    .then( () => setDiscoverable(aAdapter, true) )
> > +    .then( () => setName(aAdapter, EMULATOR_NAME) )
> > +    .then( () => setDiscoverable(aAdapter, false) )
> > +    .then( () => setDiscoverableTimeout(aAdapter, 120) );
> 
> Question: the order of calling setDiscoverable() and
> setDiscoverableTimeout() are upside down. Do you tend to do this?
> 
> Moreover, since we won't have setDiscoverableTimeout() API in new
> WebBluetooth API design, I would like to suggest that we should remove
> setting this property in our test case.

Comment 28

[Jamin Liu [:jaliu][:Jamin]]

Attachment: [Mnw] Add test cases to set/get properties of BluetoothAdapter. r=vyang

Fix typo.

Comment 29

[Jamin Liu [:jaliu][:Jamin]]

Attachment: (final) [Mnw] Add test cases to set/get properties of BluetoothAdapter. r=vyang , r=echou

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/44b695069ce7

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/44b695069ce7

Comment 32

[Firefox Commit Bot]

The following changeset is now in Firefox Nightly:

> 44b695069ce7 Bug 860697 - B2G Bluetooth: Add test cases to set/get properties of BluetoothAdapter. r=vyang, r=echou

Nightly Build Information:

        ID: 20140402030201
 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central

Download Links:

>         Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2
>      Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2
> Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2
>               Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg
>             Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe
>             Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe

Previous Nightly Build Information:

        ID: 20140401030203
 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central