WebTelephony: investigate test failures for outgoing calls

RESOLVED FIXED in mozilla16

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: philikon, Assigned: hsinyi)

Tracking

Trunk
mozilla16
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Some of the outgoing call tests in bug 756607 fail and had to be disabled. We should investigate if they're testing faulty assumptions, our implementation is busted, or the emulator is messing things up somehow.
Assignee

Updated

7 years ago
Assignee: nobody → htsai
Not a blocker - a test harness problem.
No longer blocks: webtelephony
blocking-basecamp: --- → -
Assignee

Comment 2

7 years ago
After discussing with Price about the error-notifying mechanism and some experiments, "error" event is never fired in the situation of dialing a bad number. That's the reason why "test_outgoing_badNumber.js" in bug 756607 always fails. 

Actually, the events of dialing a bad number are the same as those of dialing a legal number.
Assignee

Comment 3

7 years ago
Regarding the failure in "test_outgoing_busy.js" there are two reasons: 
(1) WebTelephony never fires "busy" event because the call state never reaches nsIRadioInterfaceLayer::CALL_STATE_BUSY in our implementation. 
(2) the emulator command "gsm busy" can be used only if the current call state is "waiting". In the test script, the call state right before "gsm busy" is unknown.
Assignee

Comment 4

7 years ago
"gsm cancel" cannot be used to cancel a call with unknown state. We need to wait until the connection is established, then "gsm cancel" works fine.
Attachment #630106 - Flags: review?(philipp)
Assignee

Comment 5

7 years ago
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #3)
> Regarding the failure in "test_outgoing_busy.js" there are two reasons: 
> (1) WebTelephony never fires "busy" event because the call state never
> reaches nsIRadioInterfaceLayer::CALL_STATE_BUSY in our implementation. 

I filed Bug 761533 for this failure.
Assignee

Comment 6

7 years ago
addressed some nits
Attachment #630106 - Attachment is obsolete: true
Attachment #630106 - Flags: review?(philipp)
Attachment #630112 - Flags: review?(philipp)
Assignee

Comment 7

7 years ago
Minor modifications are applied according to the investigation in bug 757587. Thanks.
Attachment #630112 - Attachment is obsolete: true
Attachment #630112 - Flags: review?(philipp)
Attachment #631317 - Flags: review?(philipp)
Assignee

Comment 8

7 years ago
Posted patch Patch (v4) (obsolete) — Splinter Review
Updated.

When I was working on bug 761533, I found that we might miss something for call error event. Seems nothing incorrect in Philipp's test script 'test_outgoing_basNumber.js' So, I added 'test_outgoing_basNumber.js', which had been deleted in attachment 631317 [details] [diff] [review], back. Thanks.
Attachment #631317 - Attachment is obsolete: true
Attachment #631317 - Flags: review?(philipp)
Attachment #631341 - Flags: review?(philipp)
Assignee

Comment 9

7 years ago
In Bug 761533, i will revisit call error handling function. So, I will keep investigate test_outgoing_badNumber and test_outgoing_busy there.
Comment on attachment 631341 [details] [diff] [review]
Patch (v4)

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

Good sleuthing! r=me with readability nit addressed.

::: dom/telephony/test/marionette/test_outgoing_reject.js
@@ +75,5 @@
> +function waitForCancel() {
> +  runEmulatorCmd("gsm list", cmdCallback);
> +}
> +
> +function cmdCallback(result) {

Not sure why you didn't inline this... You don't refer to it anywhere else, so it just clutters up readability, I think.
Attachment #631341 - Flags: review?(philipp) → review+
Assignee

Comment 11

7 years ago
(In reply to Philipp von Weitershausen [:philikon] from comment #10)
> Comment on attachment 631341 [details] [diff] [review]
> Patch (v4)
> 
> Review of attachment 631341 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Good sleuthing! r=me with readability nit addressed.
> 
> ::: dom/telephony/test/marionette/test_outgoing_reject.js
> @@ +75,5 @@
> > +function waitForCancel() {
> > +  runEmulatorCmd("gsm list", cmdCallback);
> > +}
> > +
> > +function cmdCallback(result) {
> 
> Not sure why you didn't inline this... You don't refer to it anywhere else,
> so it just clutters up readability, I think.
Thanks for the comment. Will provide a new patch with the comment addressed. :)
Assignee

Comment 12

7 years ago
Posted patch Patch (v5)Splinter Review
addressed nit in Comment #10. Thanks!
Attachment #631341 - Attachment is obsolete: true
Assignee

Updated

7 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/a958b5ab2f48
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/a958b5ab2f48
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.