test_settings_bluetooth.py:test_toggle_bluetooth_settings is failing on bluetooth_settings.enable_visible_to_all()

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: onelson, Assigned: njpark)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [3.0-Daily-Testing], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8566043 [details]
2015-02-18-08-38-57.png

Description:
test_settings_bluetooth.py is failing consistently today, 7/20 via local automation runs. The case appears to be setting it's visibility to to all to 'off' directly after renaming the device; this causes it to hang and timeout.

Repro Steps:
gaiatest --testvars=gaiatest/testvars_smoke.json --address=localhost:2828 --timeout=30000 --repeat=9 --restart --html-output=feb18_bluetooth2.html gaiatest/tests/functional/settings/test_settings_bluetooth.py

Traceback (most recent call last):
  File "/home/flash/Desktop/oliverthor/git/gaia/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.8.7-py2.7.egg/marionette/marionette_test.py", line 283, in run
    testMethod()
  File "/home/flash/Desktop/oliverthor/git/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/settings/test_settings_bluetooth.py", line 37, in test_toggle_bluetooth_settings
    bluetooth_settings.enable_visible_to_all()
  File "/home/flash/Desktop/oliverthor/git/gaia/tests/python/gaia-ui-tests/gaiatest/apps/settings/regions/bluetooth.py", line 54, in enable_visible_to_all
    Wait(self.marionette).until(expected.element_selected(checkbox))
  File "/home/flash/Desktop/oliverthor/git/gaia/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/marionette_client-0.8.7-py2.7.egg/marionette/wait.py", line 143, in until
    cause=last_exc)
TimeoutException: TimeoutException: Timed out after 30.0 seconds

Environmental Variables:
Device firmware (base)	L1TC100118D0
Device firmware (date)	18 Feb 2015 01:45:09
Device firmware (incremental)	eng.cltbld.20150218.044458
Device firmware (release)	4.4.2
Device identifier	flame
Gaia date	17 Feb 2015 15:32:21
Gaia revision	82f286f10a41
Gecko build	20150218010226
Gecko revision	9696d1c4b3ba
Gecko version	38.0a1

Reproducible manually: No

Repro frequency: 7/20
(Reporter)

Comment 1

4 years ago
Build Failure:
http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.nightly.ui.functional.smoke/37/
QA Whiteboard: [QAnalyst-Triage?][fxosqa-auto-backlog?]
Flags: needinfo?(pbylenga)
Whiteboard: [3.0-Daily-Testing]
I changed this file recently in bug 1118219.
I've run this locally 11 times and it fails with this failure 2 times.
I noticed when this failure happens, the "Visible to all" checkbox is getting checked almost immediately after the Bluetooth checkbox is checked.
So when enable_visible_to_all() is called, the "Visible to all" checkbox is getting unchecked again, because that function does manually tap on it, which then causes it to get unchecked.

I don't see how that can be a bug in the test. When bluetooth is enabled, bluetooth visibility shouldn't suddenly be enabled too.
Flags: needinfo?(jaliu)
I wonder about this code:
http://mxr.mozilla.org/gaia/source/apps/bluetooth/js/settings.js#173
When lock().get fails, the "Visible to all" checkbox is checked, why is that? Although I can't see in what situation a lock().get() would fail.
QA Whiteboard: [QAnalyst-Triage?][fxosqa-auto-backlog?] → [QAnalyst-Triage+][fxosqa-auto-backlog?]
Flags: needinfo?(pbylenga)
(In reply to Martijn Wargers [:mwargers] (QA) from comment #2)
> I changed this file recently in bug 1118219.
> I've run this locally 11 times and it fails with this failure 2 times.
> I noticed when this failure happens, the "Visible to all" checkbox is
> getting checked almost immediately after the Bluetooth checkbox is checked.
> So when enable_visible_to_all() is called, the "Visible to all" checkbox is
> getting unchecked again, because that function does manually tap on it,
> which then causes it to get unchecked.
> 
> I don't see how that can be a bug in the test. When bluetooth is enabled,
> bluetooth visibility shouldn't suddenly be enabled too.

Hi Martijn,
Thank you for the information.
It seems like it hit Bug 1098228.

Hi Ian,
Would you mind to take a look and check if this bug is caused by Bug 1098228 ?
Thanks.
Flags: needinfo?(jaliu) → needinfo?(iliu)
See Also: → bug 1098228
This started failing frequently with http://jenkins1.qa.scl3.mozilla.com/view/Mozilla%20Lab/job/flame-kk-319.mozilla-central.tinderbox.ui.functional.smoke/366/ , Feb 13, 2015 11:21:49 AM (PST)
Summary: test_settings_bluetooth.py is failing on bluetooth_settings.enable_visible_to_all() → test_settings_bluetooth.py:test_toggle_bluetooth_settings is failing on bluetooth_settings.enable_visible_to_all()
Just checked back in other jobs, and failure rate is around 50%. That's noisy enough to disable while investigating, which I'm doing in bug 1141124.
(In reply to Martijn Wargers [:mwargers] (QA) from comment #3)
> I wonder about this code:
> http://mxr.mozilla.org/gaia/source/apps/bluetooth/js/settings.js#173
> When lock().get fails, the "Visible to all" checkbox is checked, why is
> that? Although I can't see in what situation a lock().get() would fail.

Jamin, do you know the answer to this?
Flags: needinfo?(jaliu)
Blocks: 1141124
Created attachment 8574699 [details] [review]
[gaia] geoelectric:disable-1134244 > mozilla-b2g:master
Comment on attachment 8574699 [details] [review]
[gaia] geoelectric:disable-1134244 > mozilla-b2g:master

This was placed in here in error. Will amend to go to 1141124 as intended.
Attachment #8574699 - Attachment is obsolete: true
(In reply to Martijn Wargers [:mwargers] (QA) from comment #7)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #3)
> > I wonder about this code:
> > http://mxr.mozilla.org/gaia/source/apps/bluetooth/js/settings.js#173
> > When lock().get fails, the "Visible to all" checkbox is checked, why is
> > that? Although I can't see in what situation a lock().get() would fail.
> 
> Jamin, do you know the answer to this?

Ian,
Do you know why we set "visibleCheckBox.checked = true" when lock().get fails ?

> // get last user setting for device visible
> var req = settings.createLock().get('bluetooth.visible');
> ...
> req.onerror = function bt_getVisibleError() {
>   visibleCheckBox.checked = true;
> };
Flags: needinfo?(jaliu)
(In reply to Jamin Liu [:jaliu] from comment #4)
> Hi Ian,
> Would you mind to take a look and check if this bug is caused by Bug 1098228
> ?
> Thanks.
The problem here we hit, is absolutely same with bug 1098228. I have mentioned in link(https://bugzilla.mozilla.org/show_bug.cgi?id=1098228#c6).
(In reply to Jamin Liu [:jaliu] from comment #10)
> Ian,
> Do you know why we set "visibleCheckBox.checked = true" when lock().get
> fails ?
> 
> > // get last user setting for device visible
> > var req = settings.createLock().get('bluetooth.visible');
> > ...
> > req.onerror = function bt_getVisibleError() {
> >   visibleCheckBox.checked = true;
> > };

I trace the code twice. I still can not understand why given 'true' state to the visible check box while the request fall into onerror case. And this line is not the root cause for disable visible immediately while Bluetooth is from OFF -> ON.

This is an old code base from v1.0.(https://github.com/mozilla-b2g/gaia/commit/4ad40bcedfac4bcc29f1ffd54f0c9547d243e44e).

Only one thing I can understand that the patch wants to get user's preference 'bluetooth.visible'. Then, use the preference to restore discoverable state. For initial state, it uses platform discoverable state to init the visible check box. 


Most of the case is as following:

STR:
0) Enable Bluetooth and set visible to be true.
1) Disable Bluetooth.
2) Enable Bluetooth.

Actual result:
Will see the visible state from enabled to disabled.

The root cause is the user's preference not sync with platform value in this use case. So we can see the visible checked box be changed from checked to unchecked.

How we can do for the problem.
1). Grey out the state until we set the user's preference to platform with onsuccess(). This is risk if platform always set discoverable enabled in failed case.
2). Let the default value of visible to be false. And no longer to restore state for user's preference.

I would like solution 2) because I agree with comment 2. Shall we fix the problem immediately? My concern is having new user story in bug 1121913.

New feature: visible timer (Settings app) - https://bugzilla.mozilla.org/show_bug.cgi?id=1121913
Flags: needinfo?(iliu)
Per comment 12, ni Howie for decision.
Flags: needinfo?(hochang)
Note that this test, [test_settings_bluetooth.py, is currently not running in daily automated testing, so if things regress even further, it won't get noticed (at least, not quickly).

Comment 15

4 years ago
not high priority, and bug 1121913 will solve this
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(hochang)
Resolution: --- → DUPLICATE
Duplicate of bug: 1121913
Reopening, this bug is different than bug 1121913. I know comment 15 is saying that bug 1121913 is fixing this, but I don't see any movement in that bug.
Status: RESOLVED → REOPENED
Depends on: 1121913
Resolution: DUPLICATE → ---
Created attachment 8655666 [details] [review]
[gaia] npark-mozilla:1134244 > mozilla-b2g:master
(Assignee)

Comment 18

3 years ago
I made changes to the bluetooth.py.  Basically, when visible_to_all is enabled, I did not see any changes to detect on webIDE.  The switch element is hidden, and doesn't show whether it's checked or not, so I am checking the bluetooth visibility from the data_layer to confirm the changes.

Also had to add disable_bluetooth() methods and change the locator for the Bluetooth switch.
(Assignee)

Updated

3 years ago
Attachment #8655666 - Flags: review?(martijn.martijn)
Attachment #8655666 - Flags: review?(jlorenzo)
(Assignee)

Updated

3 years ago
Assignee: nobody → npark
(Assignee)

Comment 19

3 years ago
BTW Comment 2 does not seem to be reproducing with the latest build
(In reply to No-Jun Park [:njpark] from comment #19)
> BTW Comment 2 does not seem to be reproducing with the latest build

I guess you repeated the test 10 times?
Comment on attachment 8655666 [details] [review]
[gaia] npark-mozilla:1134244 > mozilla-b2g:master

I think you also need to check for the checkboxes to ensure that they are checked/unchecked when tapping on them. I don't think you need to do this Wait(self.marionette).until(lambda m: data_layer.bluetooth_is_discoverable is True) thing you're doing.
If you think this test is then passing afterwards and running stable (after a lot of repeats, you should remove the stable = false rule from the manifest.ini file).
Attachment #8655666 - Flags: review?(martijn.martijn) → review-
(Assignee)

Comment 22

3 years ago
Comment on attachment 8655666 [details] [review]
[gaia] npark-mozilla:1134244 > mozilla-b2g:master

inserted the check for gaia-switch now
Attachment #8655666 - Flags: review- → review?(martijn.martijn)
Comment on attachment 8655666 [details] [review]
[gaia] npark-mozilla:1134244 > mozilla-b2g:master

Thanks for looking into this! The patch looks great.
I repeated the test 10 times and it passed every time, so this seems indeed stable again.

(In reply to Martijn Wargers [:mwargers] (QA) from comment #16)
> Reopening, this bug is different than bug 1121913. I know comment 15 is
> saying that bug 1121913 is fixing this, but I don't see any movement in that
> bug.

I guess that with the move to bluetooth_v2, this test got stable again.
Anyway, it doesn't matter, it's important that we get this test running again.
Attachment #8655666 - Flags: review?(martijn.martijn) → review+
Comment on attachment 8655666 [details] [review]
[gaia] npark-mozilla:1134244 > mozilla-b2g:master

LGTM, modulo the unused function, disable_bluetooth().

I made a comment about the usage of PageRegion. That's fine if we don't implement it in this PR.
Attachment #8655666 - Flags: review?(jlorenzo) → review+
(Assignee)

Comment 25

3 years ago
Unused function is removed, and PageRegion is used.  Ran 10 times with no failures.


Merged:
https://github.com/mozilla-b2g/gaia/commit/ac07d7fc6b36c5852d58f4e1bcbfca9ac664b2d3
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.