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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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?

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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?

Comment 3

[Philipp von Weitershausen [:philikon]]

(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.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: 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.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Attachment: 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.

Comment 6

[Philipp von Weitershausen [:philikon]]

(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 7

[Philipp von Weitershausen [:philikon]]

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!

Comment 8

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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.

Comment 9

[Philipp von Weitershausen [:philikon]]

(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.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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!

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/5c3fea9f6da6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5c3fea9f6da6