Closed Bug 1035652 (webbt-test-discovery) Opened 6 years ago Closed 6 years ago

Write marionette tests for start/stop discovery

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S6 (18july)

People

(Reporter: ben.tian, Assigned: jaliu)

References

Details

(Whiteboard: [webbt-api])

Attachments

(1 file, 2 obsolete files)

Write marionette tests for BluetoothAdapter start/stopDiscovery and BluetoothDiscoveryHandle API.
Jamin, please help on this bug. Thanks.
Assignee: nobody → jaliu
Alias: webbt-test-discovery
Target Milestone: --- → 2.0 S6 (18july)
Status: NEW → ASSIGNED
nit: Add few white spaces for readability.
Attachment #8453626 - Attachment is obsolete: true
Attachment #8453626 - Flags: review?(btian)
Attachment #8453639 - Flags: review?(btian)
Bug 1037291 is required to pass this test Attachment #8453639 [details] [diff] at step [4] and step [8].
Depends on: 1037291
Comment on attachment 8453639 [details] [diff] [review]
Write a marionette test for verifying the setters of adapter based on bluetooth API v2. (v2)

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

r=me with nits addressed. Note the latest code doesn't resolve stopDiscovery when adapter is not discovering. I'll file another bug to solve it.

::: dom/bluetooth2/tests/marionette/head.js
@@ +563,5 @@
>  
>  /**
> + * Wait for specified number of 'devicefound' events.
> + *
> + * Resolve if specified number of devices has been found.  Never reject.

nit: remove extra space before Never.

@@ +572,5 @@
> + * @param aDiscoveryHandle
> + *        A BluetoothDiscoveryHandle which is used to notify application of
> + *        discovered remote bluetooth devices.
> + * @param aExpectedNumberOfDevices
> + *        The number of remote devices we expect to discovery.

nit: 'discover'

::: dom/bluetooth2/tests/marionette/test_dom_BluetoothAdapter_discovery_API2.js
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +///////////////////////////////////////////////////////////////////////////////
> +// Test Purpose:
> +//   To verify the discovery process of BluetoothAdapter is correct.

nit: remove 'is correct'. 'verify' already means to show it's correct.

@@ +4,5 @@
> +
> +///////////////////////////////////////////////////////////////////////////////
> +// Test Purpose:
> +//   To verify the discovery process of BluetoothAdapter is correct.
> +//   Tester have to put the B2G devices to an environment which is surrounded by

nit: Tester's' ... put B2G devices 'in' an environment

@@ +6,5 @@
> +// Test Purpose:
> +//   To verify the discovery process of BluetoothAdapter is correct.
> +//   Tester have to put the B2G devices to an environment which is surrounded by
> +//   N discoverable remote devices. To pass this test, the number N has to be
> +//   greater than EXPECTED_NUMBER_OF_REMOTE_DEVICES.

'be greater than or equals to'

@@ +68,5 @@
> +        }
> +      };
> +    })
> +    .then(function() {
> +      log("[3] Stop discovery and and verify the correctness ... ");

nit: remove extra 'and'.

@@ +77,5 @@
> +      promises.push(aAdapter.stopDiscovery());
> +      return Promise.all(promises);
> +    })
> +    .then(function() {
> +      log("[4] Assume the BluetoothDiscoveryHandle from [1] has expired ... ");

I suggest to use "Mark the BluetoothDiscoveryHandle from [1] as expired" instead of "Assume...". "Assume" sounds like stating pre-condition rather than doing action.

@@ +105,5 @@
> +      log("[8] Call 'startDiscovery' twice continuously ... ");
> +      return aAdapter.startDiscovery()
> +        .then(() => aAdapter.startDiscovery())
> +        .then(() => ok(false, "Call startDiscovery() when it's discovering. - Fail"),
> +              () => ok(true, "Call startDiscovery() when it's discovering. - Success"));

nit: ... when 'adapter' is discovering

@@ +112,5 @@
> +      log("[9] Call 'stopDiscovery' twice continuously ... ");
> +      return aAdapter.stopDiscovery()
> +        .then(() => aAdapter.stopDiscovery())
> +        .then(() => ok(true, "Call stopDiscovery() when it's not discovering. - Success"),
> +              () => ok(false, "Call stopDiscovery() when it's not discovering. - Fail"));

Ditto.
Attachment #8453639 - Flags: review?(btian) → review+
Depends on: 1038053
Revise based on reviewer's comment.
Attachment #8453639 - Attachment is obsolete: true
Attachment #8455233 - Flags: review?(btian)
(In reply to Ben Tian [:btian] from comment #5)
Thank you for your comments. :)
Attachment #8455233 - Flags: review?(btian)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/81eedce71bbf
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.