Closed Bug 942218 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
RESOLVED FIXED
Milestone:
1.3 Sprint 6 - 12/6
Tracking Flags:
Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 --- fixed
firefox-esr24 --- unaffected
b2g-v1.2 --- unaffected

People

(Reporter: RyanVM, Assigned: aknow)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

[Final] Fix isRadioChanging. r=hsinyi
3.22 KB, patch
aknow
: review+
Details | Diff | Splinter Review 
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)
Assignee: nobody → szchen
I know the problem. It's indeed a bug. Fixing now
Attached patch fix isRadioChanging (obsolete) — — Splinter Review 
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 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)
(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
Attached patch #2 fix isRadioChanging (obsolete) — — Splinter Review 

        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+

    
    

    
  


  

        Attachment #8338307 -
        Attachment is obsolete: true

        Attachment #8338394 -
        Flags: review+

    
    

    
      
  

    
    

    
      
  

    
  
Keywords: checkin-needed

    
    

    
      
  

    
    

    
      
  

    
  

          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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: