Closed Bug 1128821 Opened 9 years ago Closed 9 years ago

[Telephony] Marionette Test: Ensure radio on before test

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(tracking-b2g:backlog, firefox39 fixed)

RESOLVED FIXED
2.2 S7 (6mar)
tracking-b2g backlog
Tracking Status
firefox39 --- fixed

People

(Reporter: aknow, Assigned: aknow)

Details

Attachments

(1 file)

      No description provided.
Attachment #8558309 - Flags: review?(htsai)
Comment on attachment 8558309 [details] [diff] [review]
Ensure radio on before test

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

::: dom/telephony/test/marionette/head.js
@@ +1167,5 @@
>      SpecialPowers.setBoolPref(kPrefRilDebuggingEnabled, true);
>      log("Set debugging pref: " + debugPref + " => true");
>  
> +    return Promise.resolve()
> +      .then(ensureRadio)

Just curious: did you encounter some radio problems when running marionette-webapi tests? I was asking because the default radio state is supposed to be on.

@@ -1152,5 @@
>      function tearDown() {
>        log("== Test TearDown ==");
>        emulator.waitFinish()
>          .then(() => {
> -          permissionTearDown();

Why removed this?

If this is not necessary, please also clean up [1].

[1] https://dxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/head.js?from=telephony/test/marionette/head.js#1121
Attachment #8558309 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> Comment on attachment 8558309 [details] [diff] [review]
> Ensure radio on before test
> 
> Review of attachment 8558309 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/telephony/test/marionette/head.js
> @@ +1167,5 @@
> >      SpecialPowers.setBoolPref(kPrefRilDebuggingEnabled, true);
> >      log("Set debugging pref: " + debugPref + " => true");
> >  
> > +    return Promise.resolve()
> > +      .then(ensureRadio)
> 
> Just curious: did you encounter some radio problems when running
> marionette-webapi tests? I was asking because the default radio state is
> supposed to be on.

Sometimes, there will be a failure or timeout when performing the radio related test.  Then, for the next test, the radio may not be in a good state that and it results in another failure.

> @@ -1152,5 @@
> >      function tearDown() {
> >        log("== Test TearDown ==");
> >        emulator.waitFinish()
> >          .then(() => {
> > -          permissionTearDown();
> 
> Why removed this?

What pushPermission do is

  /* apply permissions to the system and when the test case is finished (SimpleTest.finish())
     we will revert the permission back to the original.

Therefore, don't worry about it.  The permission will be reset.

> 
> If this is not necessary, please also clean up [1].
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/
> head.js?from=telephony/test/marionette/head.js#1121

The part you mentioned is indeed removed in the patch.  Did I miss anything?
(In reply to Szu-Yu Chen [:aknow] from comment #3)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> > Comment on attachment 8558309 [details] [diff] [review]
> > Ensure radio on before test
> > 
> > Review of attachment 8558309 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: dom/telephony/test/marionette/head.js
> > @@ +1167,5 @@
> > >      SpecialPowers.setBoolPref(kPrefRilDebuggingEnabled, true);
> > >      log("Set debugging pref: " + debugPref + " => true");
> > >  
> > > +    return Promise.resolve()
> > > +      .then(ensureRadio)
> > 
> > Just curious: did you encounter some radio problems when running
> > marionette-webapi tests? I was asking because the default radio state is
> > supposed to be on.
> 
> Sometimes, there will be a failure or timeout when performing the radio
> related test.  Then, for the next test, the radio may not be in a good state
> that and it results in another failure.
> 
> > @@ -1152,5 @@
> > >      function tearDown() {
> > >        log("== Test TearDown ==");
> > >        emulator.waitFinish()
> > >          .then(() => {
> > > -          permissionTearDown();
> > 
> > Why removed this?
> 
> What pushPermission do is
> 
>   /* apply permissions to the system and when the test case is finished
> (SimpleTest.finish())
>      we will revert the permission back to the original.
> 
> Therefore, don't worry about it.  The permission will be reset.
> 
> > 
> > If this is not necessary, please also clean up [1].
> > 
> > [1]
> > https://dxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/
> > head.js?from=telephony/test/marionette/head.js#1121
> 
> The part you mentioned is indeed removed in the patch.  Did I miss anything?

Oops, forgive me :)
blocking-b2g: --- → backlog
https://hg.mozilla.org/mozilla-central/rev/7f83733e96cb
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S7 (6mar)
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: