The default bug view has changed. See this FAQ.

WebTelephony: add tests for hanging up dialing/alerting/holding calls

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hsinyi, Assigned: hsinyi)

Tracking

unspecified
mozilla16
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Assignee: nobody → htsai
(Assignee)

Comment 1

5 years ago
When writing the test, i realized that it is ignored to hang up a 'holding' or 'held' call. The original thought was that a held call should be resumed first before being hung up.

I am thinking maybe we would like to change this situation here?
(Assignee)

Comment 2

5 years ago
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> When writing the test, i realized that it is ignored to hang up a 'holding'
> or 'held' call. The original thought was that a held call should be resumed
> first before being hung up.
> 
> I am thinking maybe we would like to change this situation here?

Also, remove 'held' call from active call, if it was be?
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> When writing the test, i realized that it is ignored to hang up a 'holding'
> or 'held' call. The original thought was that a held call should be resumed
> first before being hung up.

If it's possible to hang up a held call, we should support it in the API. Otherwise we should probably throw an exception to indicate that our phone stack doesn't support it.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> Also, remove 'held' call from active call, if it was be?

Yes.
Created attachment 638979 [details] [diff] [review]
patch

Here are tests for hanging up alerting and held calls. However, seems that emulator does not support the request 'RIL_LAST_CALL_FAIL_CAUSE' that we use to make call into 'disconnected', so the tests fail in capturing 'disconnected' event. 
If we revert back before https://hg.mozilla.org/mozilla-central/rev/f7e10a29e5c6 the tests work fine.

Same situation to all other telephony tests. So I disable them all at this moment.
Attachment #638979 - Flags: review?(philipp)
Attachment #638979 - Attachment is obsolete: true
Attachment #638979 - Flags: review?(philipp)
(Assignee)

Comment 5

5 years ago
Created attachment 638982 [details] [diff] [review]
patch (v2)

Here are tests for hanging up alerting and held calls. However, seems that emulator does not support the request 'RIL_LAST_CALL_FAIL_CAUSE' that we use to make call into 'disconnected', so the tests fail in capturing 'disconnected' event. 
If we revert back before https://hg.mozilla.org/mozilla-central/rev/f7e10a29e5c6 the tests work fine.

Same situation to all other telephony tests. So I disable them all at this moment.
Attachment #638982 - Flags: review?(philipp)
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5)
> Here are tests for hanging up alerting and held calls. However, seems that
> emulator does not support the request 'RIL_LAST_CALL_FAIL_CAUSE' that we use
> to make call into 'disconnected', so the tests fail in capturing
> 'disconnected' event. 
> If we revert back before
> https://hg.mozilla.org/mozilla-central/rev/f7e10a29e5c6 the tests work fine.

I see! So that's the reason for bug 768608! The solution here is to make sure we still do the right thing even if the RIL doesn't support RIL_LAST_CALL_FAIL_CAUSE. Let's do that in bug 768608, I'll assign it to you, ok?
Comment on attachment 638982 [details] [diff] [review]
patch (v2)

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

Looks good to me. But please don't disable the tests; we should rather fix them. (This also applies to the comments in the tests.) r=me with that addressed.

Thanks!
Attachment #638982 - Flags: review?(philipp) → review+
(Assignee)

Comment 8

5 years ago
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Comment on attachment 638982 [details] [diff] [review]
> patch (v2)
> 
> Review of attachment 638982 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me. But please don't disable the tests; we should rather fix
> them. (This also applies to the comments in the tests.) r=me with that
> addressed.
> 
> Thanks!
Hi Philipp,

Are you saying that I should remove the comments in the tests as well? Thanks.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #8)
> Are you saying that I should remove the comments in the tests as well?

Yes, since they no longer apply once we make those tests pass.
(Assignee)

Comment 10

5 years ago
(In reply to Philipp von Weitershausen [:philikon] from comment #9)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #8)
> > Are you saying that I should remove the comments in the tests as well?
> 
> Yes, since they no longer apply once we make those tests pass.

Got it. Will address your comments. Thanks!
(Assignee)

Comment 11

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/5c3fea9f6da6
https://hg.mozilla.org/mozilla-central/rev/5c3fea9f6da6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.