Intermittent test_mobile_set_radio.js | got true, expected false | got false, expected true

RESOLVED FIXED in Firefox 28

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: aknow)

Tracking

({intermittent-failure})

unspecified
1.3 Sprint 6 - 12/6
ARM
Gonk (Firefox OS)
intermittent-failure

Firefox Tracking Flags

(firefox26 unaffected, firefox27 unaffected, firefox28 fixed, firefox-esr24 unaffected, b2g-v1.2 unaffected)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=30965924&tree=Mozilla-Inbound

b2g_emulator_vm mozilla-inbound opt test marionette-webapi on 2013-11-22 09:09:22 PST for push 4de097a9f7a6
slave: tst-linux64-ec2-093

09:30:18     INFO -  TEST-START test_mobile_set_radio.js
09:30:22     INFO -  /builds/slave/test/build/tests/marionette/tests/dom/network/tests/marionette/test_mobile_set_radio.js, runTest (marionette_test.MarionetteJSTestCase) ... FAIL
09:30:23     INFO -  START LOG:
09:30:23     INFO -  INFO TEST-START: /builds/slave/test/build/tests/marionette/tests/dom/network/tests/marionette/test_mobile_set_radio.js Fri Nov 22 2013 12:30:18 GMT-0500 (EST)
09:30:23     INFO -  INFO = testSwitchRadio = Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled to false Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: disabling Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: disabled Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled success Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled to true Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: enabling Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: enabled Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled success Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO = testDisableRadioWhenDataConnected = Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO Turn data on. Fri Nov 22 2013 12:30:19 GMT-0500 (EST)
09:30:23     INFO -  INFO mobileConnection.data.connected is now 'true'. Fri Nov 22 2013 12:30:21 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled to false Fri Nov 22 2013 12:30:21 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: disabling Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: disabled Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled success Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled to true Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: enabling Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO Received 'radiostatechange' event, radioState: enabled Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  INFO setRadioEnabled success Fri Nov 22 2013 12:30:22 GMT-0500 (EST)
09:30:23     INFO -  END LOG:
09:30:23     INFO -  ======================================================================
09:30:23     INFO -  FAIL: None
09:30:23     INFO -  ----------------------------------------------------------------------
09:30:23     INFO -  Traceback (most recent call last):
09:30:23     INFO -    File "/builds/slave/test/build/tests/marionette/marionette/marionette_test.py", line 143, in run
09:30:23     INFO -      testMethod()
09:30:23     INFO -    File "/builds/slave/test/build/tests/marionette/marionette/marionette_test.py", line 436, in runTest
09:30:23     INFO -      '%d tests failed:\n%s' % (results['failed'], '\n'.join(fails)))
09:30:23     INFO -  AssertionError: 1 tests failed:
09:30:23     INFO -  TEST-UNEXPECTED-FAIL | test_mobile_set_radio.js | got true, expected false | got false, expected true
09:30:23     INFO -  ----------------------------------------------------------------------
09:30:23     INFO -  Ran 1 test in 5.521s
09:30:23     INFO -  FAILED (failures=1)
(Reporter)

Updated

4 years ago
Assignee: nobody → szchen
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 7

4 years ago
I know the problem. It's indeed a bug. Fixing now
(Assignee)

Comment 8

4 years ago
Created attachment 8337639 [details] [diff] [review]
fix isRadioChanging

bug in original isRadioChanging()

when set radio off
1. deactivating data calls .. done
2. currently, we are not yet sent out the setRadioEnable request. so the state is not changed now.
3. isRadioChanging() return false.
4. radio interface will try to re-establish the data call because it is not in radioChanging.

data may be reconnected between (a) deactive data call and (b) send out setRadioEnabled.
Attachment #8337639 - Flags: review?(htsai)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 8337639 [details] [diff] [review]
fix isRadioChanging

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1614,5 @@
>    },
>  
>    _isRadioChanging: function _isRadioChanging() {
>      let state = this.rilContext.detailedRadioState;
> +    return gRadioEnabledController.isDeactivatingDataCalls() ||

Having this here doesn't make much sense as this function represents whether radio is changing. 

I think we should instead add one more condition check in "updateRILNetworkInterface."
Attachment #8337639 - Flags: review?(htsai)
(Assignee)

Comment 13

4 years ago
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #12)
> Comment on attachment 8337639 [details] [diff] [review]
> fix isRadioChanging
> 
> Review of attachment 8337639 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +1614,5 @@
> >    },
> >  
> >    _isRadioChanging: function _isRadioChanging() {
> >      let state = this.rilContext.detailedRadioState;
> > +    return gRadioEnabledController.isDeactivatingDataCalls() ||
> 
> Having this here doesn't make much sense as this function represents whether
> radio is changing. 
> 
> I think we should instead add one more condition check in
> "updateRILNetworkInterface."

updateRILNetworkInterface() is the only one caller for this function.
Did you mean moving "gRadioEnabledController.isDeactivatingDataCalls()" to updateRILNetworkInterface()
So it would be
  if (gRadioEnabledController.isDeactivatingDataCalls() || this.isRadioChanging()) ...
(In reply to Szu-Yu Chen [:aknow] from comment #13)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #12)
> > Comment on attachment 8337639 [details] [diff] [review]
> > fix isRadioChanging
> > 
> > Review of attachment 8337639 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: dom/system/gonk/RadioInterfaceLayer.js
> > @@ +1614,5 @@
> > >    },
> > >  
> > >    _isRadioChanging: function _isRadioChanging() {
> > >      let state = this.rilContext.detailedRadioState;
> > > +    return gRadioEnabledController.isDeactivatingDataCalls() ||
> > 
> > Having this here doesn't make much sense as this function represents whether
> > radio is changing. 
> > 
> > I think we should instead add one more condition check in
> > "updateRILNetworkInterface."
> 
> updateRILNetworkInterface() is the only one caller for this function.
> Did you mean moving "gRadioEnabledController.isDeactivatingDataCalls()" to
> updateRILNetworkInterface()
> So it would be
>   if (gRadioEnabledController.isDeactivatingDataCalls() ||
> this.isRadioChanging()) ...

More specific, in [1]

    if (defaultDataCallState == RIL.GECKO_NETWORK_STATE_CONNECTING ||
        defaultDataCallState == RIL.GECKO_NETWORK_STATE_DISCONNECTING ||
        gRadioEnabledController.isDeactivatingDataCalls()) {
      if (DEBUG) {
        this.debug("Nothing to do during connecting/disconnecting in progress.");
      }
      return;
    }

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#1807
(Assignee)

Comment 15

4 years ago
Created attachment 8338307 [details] [diff] [review]
#2 fix isRadioChanging
Attachment #8337639 - Attachment is obsolete: true
Attachment #8338307 - Flags: review?(htsai)
Comment on attachment 8338307 [details] [diff] [review]
#2 fix isRadioChanging

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

Thank you!
Attachment #8338307 - Flags: review?(htsai) → review+
(Assignee)

Comment 17

4 years ago
Created attachment 8338394 [details] [diff] [review]
[Final] Fix isRadioChanging. r=hsinyi
Attachment #8338307 - Attachment is obsolete: true
Attachment #8338394 - Flags: review+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/912b38085d2d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
status-b2g-v1.2: --- → unaffected
status-firefox26: --- → unaffected
status-firefox27: --- → unaffected
status-firefox28: --- → fixed
status-firefox-esr24: --- → unaffected
Target Milestone: --- → 1.3 Sprint 6 - 12/6
You need to log in before you can comment on or make changes to this bug.