Closed Bug 997791 Opened 5 years ago Closed 5 years ago

Investigate and fix tests fail to enable wifi during setup

Categories

(Firefox OS Graveyard :: Wifi, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S6 (25apr)
blocking-b2g 1.3T+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: Bebe, Assigned: chucklee)

References

Details

(Keywords: regression, Whiteboard: [depend on sprd305892] )

Attachments

(5 files, 2 obsolete files)

Description:
Seems that a group of tests are failing because the setup fails to enable wifi

Environment:
Gaia      dadf0e60a6421f5b57ee9fc536c6617212805c19
Gecko     https://hg.mozilla.org/mozilla-central/rev/c55dfb01a027
BuildID   20140417040206
Version   31.0a1
ro.build.version.incremental=324
ro.build.date=Thu Dec 19 14:04:55 CST 2013


Traceback (most recent call last):
Traceback (most recent call last):
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/.env/local/lib/python2.7/site-packages/marionette_client-0.7.5-py2.7.egg/marionette/marionette_test.py", line 145, in run
self.setUp()
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/tests/python/gaia-ui-tests/gaiatest/tests/functional/settings/test_settings_power_save_mode.py", line 17, in setUp
self.data_layer.connect_to_wifi()
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/tests/python/gaia-ui-tests/gaiatest/gaia_test.py", line 269, in connect_to_wifi
self.enable_wifi()
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/tests/python/gaia-ui-tests/gaiatest/gaia_test.py", line 258, in enable_wifi
result = self.marionette.execute_async_script("return GaiaDataLayer.enableWiFi()", special_powers=True)
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/.env/local/lib/python2.7/site-packages/marionette_client-0.7.5-py2.7.egg/marionette/marionette.py", line 1162, in execute_async_script
filename=os.path.basename(frame[0]))
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/.env/local/lib/python2.7/site-packages/marionette_client-0.7.5-py2.7.egg/marionette/marionette.py", line 613, in _send_message
self._handle_error(response)
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.2/.env/local/lib/python2.7/site-packages/marionette_client-0.7.5-py2.7.egg/marionette/marionette.py", line 662, in _handle_error
raise ScriptTimeoutException(message=message, status=status, stacktrace=stacktrace)
ScriptTimeoutException: ScriptTimeoutException: timed out


Reproducible manually: NO
Attached file logcat of the issue
Eric can you take a look over this issue pls
Flags: needinfo?(echang)
I'mma gonna look at this right now because it seems quite urgent.
I can't fix this, I don't know what's wrong.

It fails enabling wifi. The setting (wifi.enabled) is set but the WifiManager never seems to pick it up and switch itself to enabled. As if the SettingsObserver never noticed the change of setting. In test_settings_powersave_mode if you comment out disable_wifi and connect_to_cell_data from the setUp steps then the test will pass fine which seems to point to it being a problem with disable and re-enable not working.

I guess this is something like the other problem we had with airplane mode recently.
I also added
self.device_manager.removeDir('/data/misc/wifi/wpa_supplicant.conf')
into the cleanup_data method as it occsaionally seems to fail to forget the networks during the restart.
Dimi, can you help us with this one?  Eric or someone from Brian's team should be able to help you get set up; thanks in advance!
Flags: needinfo?(dlee)
I will check with Dimi on this one.
Flags: needinfo?(echang)
Hi Vincent,
  This is related to the bug fixing GAIA & GECKO enable or disable infinite loop.
In testcase, following is the procedure:
1. Wifi is up after rebooting , set ignore enable to True
2. Wifi disable
3. Wifi enable, check ignore enable flag is true so do nothing
Attachment #8408771 - Flags: feedback?(vchang)
Flags: needinfo?(dlee)
Comment on attachment 8408771 [details] [diff] [review]
Patch for fixing enable disable testcase failure

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

Redirect the feedback request to Chuck. :-)
Attachment #8408771 - Flags: feedback?(vchang) → feedback?(chulee)
Comment on attachment 8408771 [details] [diff] [review]
Patch for fixing enable disable testcase failure

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

I think we can't change in this way, because those flags are preventing another settings change is applied before the "echo" coming from system app.
Like:
Wifi Enable(Settings app) -> Wifi Disable(Some unexpected behavior) -> Wifi Enable(echo from System app) -> Wifi Disable(echo from system app)

In such case, we have to ignore both next incoming enable and disable settings(echo from system app).

The problem seems to be there's no System app is Gaia UI Test to send the echo, which is not expected in normal usage.
Since we can't detect this in gecko, I am thinking of changing Gaia UI Test to make it behave like there's a system app.
Attachment #8408771 - Flags: feedback?(chulee) → feedback-
Comment on attachment 8408823 [details]
Pull Request

I will pass this onto Bebe to test the functionality.

Chuck, we definitely have the System app running when the UI Tests are running. We could set WiFi by some other method if it were preferable.

It seems a bit unusual (to  my mind) to have to set the setting twice but I don't think it would have any negative effect on the UI tests.
Attachment #8408823 - Flags: feedback?(zcampbell) → feedback?(florin.strugariu)
(In reply to Zac C (:zac) from comment #11)
> Comment on attachment 8408823 [details]
> Pull Request
> 
> I will pass this onto Bebe to test the functionality.
> 
> Chuck, we definitely have the System app running when the UI Tests are
> running. We could set WiFi by some other method if it were preferable.
> 

Yes, you are right. Then this is not cause of the problem.
I noticed that if I run the test continuously, the ignored settings might be enable or disable.
This makes be think that maybe the ignore flag is left by previous test case, maybe system app doesn't have time to set settings or something like that.
Sorry I memory massed up system app and settings app, the echo setting is done by settings app, not system app.
So gecko doesn't have to ignore next settings, instead, settings app should prevent set same value to settings.
I'll fixed them ASAP.
Assignee: nobody → chulee
Attached patch Remove ignore settings flags. (obsolete) — Splinter Review
Attachment #8408771 - Attachment is obsolete: true
Attachment #8408913 - Flags: review?(vchang)
Regression of Bug 989717
blocking-b2g: --- → 1.3T?
Depends on: 989717
Attachment #8408823 - Attachment mime type: text/plain → text/x-github-pull-request
Attachment #8408823 - Flags: feedback?(florin.strugariu)
Comment on attachment 8408823 [details]
Pull Request

Connectivity module of settings app will tend to set wifi.enabled after receives wifi enable/disable event, this behavior will cause infinite loop between gecko and gaia.
We tend to resolve the loop in gecko, but it turns out that gecko can't handle rapid settings change like bug 989717.
So it might be better to break the look in Connectivity itself.

Current pull request only checks value of |wifi.enable| before set it, but rapid setting change in |tethering.wifi.enabled| still triggers the loop.

Maybe wifi hotspot also need to block enable button before enable/disable is done.
Flags: needinfo?(arthur.chen)
This issue was also reproduced on 1.4 with automation.

Build info:
Gaia      97b8f9a0a2bd82572422dd84f84016df0e7f56c8
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/e43434b6dd4e
BuildID   20140418000201
Version   30.0a2
ro.build.version.incremental=eng.tclxa.20131223.163538
ro.build.date=Mon Dec 23 16:36:04 CST 2013

test_settings_airplane_mode.py fails intermittently with the same issue.
Component: Gaia::UI Tests → Wifi
Keywords: regression
Whiteboard: [1.4-approval-needed]
Duplicate of this bug: 998326
FYI, AreWeFastYet stopped working after this merge:
  http://hg.mozilla.org/integration/mozilla-inbound/rev/43fc4f8b6d49

This merge has some modification in dom/wifi/WifiWorker.js.
Could we just backout Bug 989717?
blocking-b2g: 1.3T? → 1.3T+
Since I have to find another way to resolve bug 989717 and the regression blocks the test, I think backout is a good solution for this bug.
Comment on attachment 8408913 [details] [diff] [review]
Remove ignore settings flags.

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

Per our discussion offline with Arthur, we could manipulate request queue to fix this. 
Cancel the review request, and wait for follow up patches.
Attachment #8408913 - Flags: review?(vchang)
Clear ni? per our offline discussion earlier.
Flags: needinfo?(arthur.chen)
Only preserve latest command in command queue, to prevent self-loop and fasten the operation.
Attachment #8409632 - Flags: review?(vchang)
Gecko patches can fix this bug, but it can't fully fix bug presented in bug 989717.
After more experiment, I concluded that bug 989717 has to be fixed both on gecko and gaia side, as provided in this bug.
Attachment #8409631 - Flags: review?(vchang) → review+
Comment on attachment 8409632 [details] [diff] [review]
0002. Apply request queue optimization.

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

Looks good.
Attachment #8409632 - Flags: review?(vchang) → review+
Comment on attachment 8408823 [details]
Pull Request

Hi Arthur,
  I found that gaia patch is still required to prevent the command loop, as comment 25. Please review this, thanks!
Attachment #8408823 - Flags: review?(arthur.chen)
Comment on attachment 8408823 [details]
Pull Request

Looks good to me. r=me with the nits addressed, thanks!
Attachment #8408823 - Flags: review?(arthur.chen) → review+
Comment on attachment 8408823 [details]
Pull Request

Sorry I had to cancel the r+ as it involves more than I thought.
Attachment #8408823 - Flags: review+
Comment on attachment 8408823 [details]
Pull Request

r=me. Please create another PR for v1.3T using Settings.getSettings as we don't have SettingsCache in v1.3T. Thanks!
Attachment #8408823 - Flags: review+
Comment on attachment 8409631 [details] [diff] [review]
0001. Remove ignore settings flags.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 989717
User impact if declined: Can't change Wifi setting after some STR
Testing completed (on m-c, etc.): m-c, 1.3T
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: No
Attachment #8409631 - Flags: approval-mozilla-aurora?
Comment on attachment 8409632 [details] [diff] [review]
0002. Apply request queue optimization.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 989717
User impact if declined: Can't change Wifi setting after some STR
Testing completed (on m-c, etc.): m-c, 1.3T
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: No
Attachment #8409632 - Flags: approval-mozilla-aurora?
Comment on attachment 8408823 [details]
Pull Request

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 989717
User impact if declined: Must be patched along with gecko part, or changing wifi setting will lead to infiniate loop
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): No
String or IDL/UUID changes made by this patch: No
Attachment #8408823 - Flags: approval-gaia-v1.4?
Attachment #8410864 - Flags: review?(arthur.chen) → review+
I think they are ready to go, note that gecko patch and gaia patch need to be merged at same time.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9cd74c2fa857
https://hg.mozilla.org/mozilla-central/rev/b44b035604b4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Comment on attachment 8408823 [details]
Pull Request

MAking an exception on 1.4 approval's here as this is a regression and needs to be fixed based on the STR
Attachment #8408823 - Flags: approval-gaia-v1.4? → approval-gaia-v1.4+
Attachment #8409631 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8409632 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Chuck Lee [:chucklee] from comment #36)
> I think they are ready to go, note that gecko patch and gaia patch need to
> be merged at same time.

Can you provide a 1.3t pull request for the gaia part?
(In reply to Fabrice Desré [:fabrice] from comment #41)
> (In reply to Chuck Lee [:chucklee] from comment #36)
> > I think they are ready to go, note that gecko patch and gaia patch need to
> > be merged at same time.
> 
> Can you provide a 1.3t pull request for the gaia part?

It's attachment 8410864 [details] [review], or I have to request approval for that pull request?
Patch for 1.3T is blocked by bug 999388, please hold the check-in until that bug is resolved by partner.
Or this patch will cause another regression.
Sorry to backout this patch (1e8ce16a1e185b11470f0595c27b8c9703e371b2), Chucklee. 

In v1.4, we didn't use requirejs in Settings app yet. 

Right now, with this patch in v1.4, connectivity.js is broken and wifi related functionalities are not able to use.

Please provide another patch or modify v1.3T patch to make it work in v1.4 ! Thanks :)
Status: RESOLVED → REOPENED
Flags: needinfo?(chulee)
Resolution: FIXED → ---
Comment on attachment 8408823 [details]
Pull Request

We should use pull request for v1.3t to gaia v1.4 instead of this one, per comment 44.
Attachment #8408823 - Flags: approval-gaia-v1.4+
Flags: needinfo?(chulee)
Comment on attachment 8410864 [details] [review]
Pull Request (1.3T)

Request approval per comment 44.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 989717
User impact if declined: Must be patched along with gecko part, or changing wifi setting will lead to infiniate loop
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): No
String or IDL/UUID changes made by this patch: No
Attachment #8410864 - Flags: approval-gaia-v1.4?
Duplicate of this bug: 1001292
Depends on: 1001292
Guys - what happened here? Some confusion on backouts & landings has resulted in the patches in this bug causing wifi to be entirely broken. Please back everything
(In reply to Jason Smith [:jsmith] from comment #48)
> Guys - what happened here? Some confusion on backouts & landings has
> resulted in the patches in this bug causing wifi to be entirely broken.
> Please back everything

Please backout everything out.
Okay - dug into this - the pvtbuild we have right now does not contain EJ's backout. I'll ask for a respin.
Depends on: 1001621
Before this patch is landed in 1.3T, we need partner to fix bug 999388 first.
Whiteboard: [depend on sprd305892]
Hi bajaj, can you help to approve approval-gaia-v1.4 per comment 46's request again ? It seems that Chunk requested gaia-v1.4 approval for wrong patch and caused the backout. Per comment 36, the gecko patch has landed to v1.4 and we need to land gaia patch as well.
Flags: needinfo?(bbajaj)
(In reply to Vincent Chang[:vchang] from comment #52)
> Hi bajaj, can you help to approve approval-gaia-v1.4 per comment 46's
> request again ? It seems that Chunk requested gaia-v1.4 approval for wrong
> patch and caused the backout. Per comment 36, the gecko patch has landed to
> v1.4 and we need to land gaia patch as well.

Please make sure to land this and help verify the 1.4 build as there has been a backout situation already due to a landing in this bug. Thanks!
Flags: needinfo?(bbajaj)
Attachment #8410864 - Flags: approval-gaia-v1.4? → approval-gaia-v1.4+
Depends on: 1004170
(In reply to bhavana bajaj [:bajaj] from comment #53)
> (In reply to Vincent Chang[:vchang] from comment #52)
> > Hi bajaj, can you help to approve approval-gaia-v1.4 per comment 46's
> > request again ? It seems that Chunk requested gaia-v1.4 approval for wrong
> > patch and caused the backout. Per comment 36, the gecko patch has landed to
> > v1.4 and we need to land gaia patch as well.
> 
> Please make sure to land this and help verify the 1.4 build as there has
> been a backout situation already due to a landing in this bug. Thanks!

I have tested on Unagi v1.4. Thanks~
We have tested tarako after bug 998326 and it seems works fine.
So please checking patches for Gaia v1.4 and Gecko/Gaia v1.3t, thanks
Keywords: checkin-needed
Flags: needinfo?(fabrice)
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #57)
> v1.4:
> https://github.com/mozilla-b2g/gaia/commit/
> b1242f33981024de59b8b4c26bacff8b876211b1
> 
> Fabrice, are you uplifting the Gecko patches to 1.3T as well?
Flags: needinfo?(fabrice)
We have no permission to land patch to gecko, Fabrice, please help.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
No longer depends on: 1004170
Blocks: MTBF-B2G
No longer blocks: MTBF-meta
You need to log in before you can comment on or make changes to this bug.