Closed Bug 1139774 Opened 8 years ago Closed 7 years ago

Investigate BT marionette-webapi test failures on emulator-kk

Categories

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

x86_64
Linux
defect

Tracking

(feature-b2g:2.5+, firefox44 fixed)

RESOLVED FIXED
FxOS-S9 (16Oct)
feature-b2g 2.5+
Tracking Status
firefox44 --- fixed

People

(Reporter: hsinyi, Assigned: wiwang)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Blocks: emulator-kk
Assign to Jamin first.

Jamin, please help check the BT KK emulator failure.
Assignee: nobody → jaliu
See Also: → 944299
We should track bug 1159625 and porting all necessary files like we did for JB emulator (bug 944299).
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.
Assign this bug to Will since he has been working on this issue.
Assignee: jaliu → wiwang
See Also: → 1175389
Priority: -- → P1
blocking-b2g: --- → 2.5+
Blocks: emu-x86-kk-tests
No longer blocks: b2g-emulator-x86-KK
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.
Remove 2.5+ as this is riding on 2.5 train only.
blocking-b2g: 2.5+ → ---
(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.
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.
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!
Attachment #8668225 - Flags: review?(shuang)
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
Attachment #8668225 - Flags: review?(shuang) → review-
Hi Shawn,

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

Could you help to review?
Thanks!
Attachment #8668225 - Attachment is obsolete: true
Attachment #8668406 - Flags: review?(shuang)
Fix a indent nit.
Attachment #8668406 - Attachment is obsolete: true
Attachment #8668406 - Flags: review?(shuang)
Attachment #8668438 - Flags: review?(shuang)
> Fix a indent nit.

s/a/an
Keywords: checkin-needed
Carry r+ from previous patch.
Attachment #8668438 - Attachment is obsolete: true
Attachment #8668451 - Flags: review+
(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.
Hi Tomcat,
Just soft reminder that we need to checkin this patch on mc. Thanks!
Flags: needinfo?(cbook)
Status: NEW → ASSIGNED
feature-b2g: --- → 2.5+
(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.
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/34ffab1aa304
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S9 (16Oct)
You need to log in before you can comment on or make changes to this bug.