Closed Bug 1006320 (webbt-test-manager) Opened 6 years ago Closed 6 years ago

Write marionette tests for BluetoothManager

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ben.tian, Assigned: jaliu)

References

Details

(Whiteboard: [webbt-api][p=1])

Attachments

(2 files, 4 obsolete files)

Write marionette tests for BluetoothManager API.
Whiteboard: [webbt-api]
No longer blocks: webbt-api-manager
Blocks: 1006306
Assignee: nobody → jaliu
The 'bluetooth2' API should pass the marionette test in #attachment 8441934 [details] [diff] [review] when Bug 1027552 land.

Please switch BT API to 'bluetooth2' by applying API configuration patch #attachment 8426731 [details] [diff] [review] which was attached in Bug 1005848 before you run this test.
Blocks: webbt-api-meta
No longer blocks: 1006306
Alias: webbt-api-test-mgr
Alias: webbt-api-test-mgr → webbt-test-manager
Whiteboard: [webbt-api] → [webbt-api][p=1]
Comment on attachment 8447896 [details] [diff] [review]
(part 1) Remove useless bluetooth API tests from bluetooth2 folder.

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

r=me with comment addressed.

::: dom/bluetooth2/tests/marionette/head.js
@@ +32,5 @@
>  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";
>  
>  // A user friendly name for remote BT device.

nit: 'An' user friendly name

@@ +47,5 @@
> + * Push required permissions and test if |navigator.mozBluetooth| exists.
> + * Resolve if it does, reject otherwise.
> + *
> + * Fulfill params:
> + *   bluetoothManager -- an reference to navigator.mozBluetooth.

nit: 'a' reference

@@ +51,5 @@
> + *   bluetoothManager -- an reference to navigator.mozBluetooth.
> + * Reject params: (none)
> + *
> + * @param aPermissions
> + *        Additional permissions to push before any test cases.  Could be either

nit: remove extra space before 'Could'

@@ +303,5 @@
>    return deferred.promise;
>  }
>  
>  /**
> + * Get the boolean value which indicates defaultAdapter of bluetooth is enabled.

nit: indicates 'whether' defaultAdapter ...

@@ +415,5 @@
>        cleanUp();
>      });
>  }
>  
>  function startBluetoothTest(aReenable, aTestCaseMain) {

Please add comment to explain this function.
Attachment #8447896 - Flags: review?(btian) → review+
Comment on attachment 8447897 [details] [diff] [review]
(part 2) Add a marionette test for verifying the functionality BluetoothManager based on bluetooth API v2.

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

r=me with nits addressed. Thanks.

::: dom/bluetooth2/tests/marionette/test_dom_BluetoothManager_API2.js
@@ +7,5 @@
> +//   To verify the basic functionality of BluetoothManager.
> +//
> +// Test Coverage:
> +//   - BluetoothManager.defaultAdapter
> +//   - BluetoothManagergetAdapters()

nit: BluetoothManager'.'getAdapters()

@@ +18,5 @@
> +
> +MARIONETTE_TIMEOUT = 60000;
> +MARIONETTE_HEAD_JS = 'head.js';
> +
> +// TODO: Listen to 'onattributechanged' when bluetooth2 API support it.

Is this TODO comment still required? Remove if it's not required anymore.
Attachment #8447897 - Flags: review?(btian) → review+
Thank you for your comments.
I've uploaded a new patch based on your comments.

(In reply to Ben Tian [:btian] from comment #8)
> > +MARIONETTE_TIMEOUT = 60000;
> > +MARIONETTE_HEAD_JS = 'head.js';
> > +
> > +// TODO: Listen to 'onattributechanged' when bluetooth2 API support it.
> 
> Is this TODO comment still required? Remove if it's not required anymore.
Thank you for reminding me.
I modified this comment and add a description for it.
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.