[Telephony] Marionette Test: Ensure radio on before test

RESOLVED FIXED in Firefox 39

Status

Firefox OS
RIL
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: aknow, Assigned: aknow)

Tracking

unspecified
2.2 S7 (6mar)
x86_64
Linux
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(tracking-b2g:backlog, firefox39 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 8558309 [details] [diff] [review]
Ensure radio on before test
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+
(Assignee)

Comment 3

4 years ago
(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
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S7 (6mar)
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.