1139774 - Investigate BT marionette-webapi test failures on emulator-kk

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect, P1)

Description

[Hsin-Yi Tsai (she/her) [:hsinyi]]

emulator-kk tests are enabled on staging treeherder https://treeherder.allizom.org/#/jobs?repo=try

I see timeout errors, e.g.
TEST-UNEXPECTED-ERROR | test_dom_BluetoothManager_enabled.js | ScriptTimeoutException: ScriptTimeoutException: timed out

TEST-UNEXPECTED-ERROR | test_dom_BluetoothManager_adapteradded.js | ScriptTimeoutException: ScriptTimeoutException: timed out

File this bug to track BT marionette-webapi test failures.

Comment 1

[Ben Tian (inactive)]

Assign to Jamin first.

Jamin, please help check the BT KK emulator failure.

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

We should track bug 1159625 and porting all necessary files like we did for JB emulator (bug 944299).

Comment 3

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

emulator-kk uses the branch 'b2g/b2g-4.4.2_r1' for 'build/' repository.

It's lack of the board config of BlueDroid since it was landed in b2g-4.3_r2.1.
https://github.com/mozilla-b2g/platform_build/pull/56/commits

Please refer to Bug 944299 for more details.

Comment 4

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

Assign this bug to Will since he has been working on this issue.

Comment 5

[Will Wang [:WillWang]]

Root cause of marionette test failure and a corresponding solution have been found for the new architecture of Bluetooth API.

This can tell the failure was from marionette test itself instead of underlying emulator(KK).

I am going to update detail here.

Comment 6

[Josh Cheng [:josh]]

Remove 2.5+ as this is riding on 2.5 train only.

Comment 7

[Will Wang [:WillWang]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0)
> emulator-kk tests are enabled on staging treeherder
> https://treeherder.allizom.org/#/jobs?repo=try
> 
> I see timeout errors, e.g.
> TEST-UNEXPECTED-ERROR | test_dom_BluetoothManager_enabled.js |
> ScriptTimeoutException: ScriptTimeoutException: timed out
> 
> TEST-UNEXPECTED-ERROR | test_dom_BluetoothManager_adapteradded.js |
> ScriptTimeoutException: ScriptTimeoutException: timed out
> 
> File this bug to track BT marionette-webapi test failures.

Since bluetooth APIv2 is already landed to m-c,
corresponding marionette test cases have been changed to followings:

1. [test_dom_BluetoothManager.js]
2. [test_dom_BluetoothAdapter_enable.js]
3. [test_dom_BluetoothAdapter_setters.js]
4. [test_dom_BluetoothDevice.js]
5. [test_dom_BluetoothAdapter_discovery.js]
6. [test_dom_BluetoothAdapter_pair.js]

For this bug, we can take the first 2 tests (1 and 2) as the related ones since the reported test cases are |test_dom_BluetoothManager_enabled.js| and |test_dom_BluetoothManager_adapteradded.js|.

Furthermore, test 1,2 are very related to the underlying emulator,
so this bug will discuss the test 1,2,
other tests (3,4,5,6) will be discussed in bug 1175389.

The detail of test 1,2 will be separately posted in following comments.

Comment 8

[Will Wang [:WillWang]]

At first, both test 1,2 were fail:

1. [test_dom_BluetoothManager.js]
2. [test_dom_BluetoothAdapter_enable.js]

After investigation, the root cause of two are same and have been identified:
- promise rejected: can't get bluetooth default adapter in time 

The solution is to add a function which will return a promise once the default adapter is created and signaled by callback |onattributechanged| of bluetooth manager.

After test, this solution can make above 2 tests successfully passed, the patch will be attached in following comment.

Comment 9

[Will Wang [:WillWang]]

Attachment: 0001-Bug-1139774-Add-a-function-to-wait-promoise-for-blue.patch

Here is the patch, which can make test 1,2 pass in 20 continuous times of test.

Note, I currently disabled all tests in the manifest.ini since landing marionette test code into m-c might need more times of test(say, continuous 100 times) for ideal stability.

----
Hi Shawn,

Could you help to review this patch?
Thanks!

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8668225 [details] [diff] [review]
0001-Bug-1139774-Add-a-function-to-wait-promoise-for-blue.patch

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

::: dom/bluetooth/tests/marionette/head.js
@@ +837,5 @@
>  
>  function startBluetoothTestBase(aPermissions, aTestCaseMain) {
>    ensureBluetoothManager(aPermissions)
> +    .then(function() {
> +      log("Wait bluetooth adapter ...");

nit:"Wait for creating bluetooth adapter..."

@@ +891,5 @@
>  }
> +
> +function waitForManagerStateChanged(aManager) {
> +  let deferred = Promise.defer();
> +  aManager.onattributechanged = function(evt) {

Why not adding some code to check evt? I think you should do it even though currently we only return one condition.

::: dom/bluetooth/tests/marionette/manifest.ini
@@ +10,2 @@
>  disabled = Bug 1175389
> +

nit: Remove empty lines.

@@ +13,2 @@
>  disabled = Bug 1175389
> +

Ditto

Comment 11

[Will Wang [:WillWang]]

Attachment: Patch: Add a function to wait promoise for bluetooth

Hi Shawn,

Thanks for your advice,
here is the revised patch, which can pass the same test above.

Could you help to review?
Thanks!

Comment 12

[Will Wang [:WillWang]]

Attachment: Patch: Add a function to wait promoise for bluetooth

Fix a indent nit.

Comment 13

[Will Wang [:WillWang]]

> Fix a indent nit.

s/a/an

Comment 14

[Will Wang [:WillWang]]

Attachment: Patch: Add a function to wait promise for bluetooth adapter creation.

Carry r+ from previous patch.

Comment 15

[Will Wang [:WillWang]]

(In reply to Will Wang [:WillWang] from comment #14)
> Created attachment 8668451 [details] [diff] [review]
> Patch: Add a function to wait promise for bluetooth adapter creation.
> 
> Carry r+ from previous patch.

Note: Set checkin-needed w/o try since this patch doesn't need it as per offline discussion.

Comment 16

[Josh Cheng [:josh]]

Hi Tomcat,
Just soft reminder that we need to checkin this patch on mc. Thanks!

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/34ffab1aa304

Comment 18

[Carsten Book [:Tomcat]]

(In reply to Josh Cheng [:josh] from comment #16)
> Hi Tomcat,
> Just soft reminder that we need to checkin this patch on mc. Thanks!

done now. sorry that checkin needed request came after my work time on friday and no one seems to have picked this up.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/34ffab1aa304