814691 - [WebAPI] WebTelephony: Develop test to verify cannot have two calls on hold simultaneously

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Robert Wood [:rwood]]

Develop a WebAPI test to verify the following WebTelephony test case:

- Have two phone calls at the same time: call 1 on hold, call 2 is active
- With call 1 still on hold, place call 2 on hold also
- Call 1 should become active automatically while call 2 is held

Test will use marionette and run on the device emulator.

Comment 1

[Robert Wood [:rwood]]

Attachment: Patch for 814691

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 684698 [details] [diff] [review]
Patch for 814691

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

Looks good. A couple of minor nits with trailing whitespace, and a remaining question over the order of arguments when asserting equality, but I'm happy to r+ this with or without these addressed as they do not affect the validity of the test.

::: dom/telephony/test/marionette/test_multiple_hold.js
@@ +51,5 @@
> +function answerIncoming() {
> +  log("Answering the incoming call.");
> +
> +  let gotConnecting = false;
> +  incomingCall.onconnecting = function onconnectingIn(event) { 

Nit: trailing whitespace

@@ +63,5 @@
> +    log("Received 'connected' call event for original (incoming) call.");
> +    is(incomingCall, event.call);
> +    is(incomingCall.state, "connected");
> +    ok(gotConnecting);
> +    is(incomingCall, telephony.active);

Most is() calls seem to be is(actual, expected) whereas if I'm not mistaken this appears to be the other way around. I'm not yet familiar enough with the framework to know if this matters here, but sometimes it can make a difference in the reporting.

@@ +115,5 @@
> +  outgoingCall = telephony.dial(outNumber);
> +  ok(outgoingCall);
> +  is(outgoingCall.number, outNumber);
> +  is(outgoingCall.state, "dialing");
> +  is(outgoingCall, telephony.active);

As previous comment with order of actual vs expected.

@@ +275,5 @@
> +      is(result[0], "OK");
> +      cleanUp();
> +    });
> +  };
> +  outgoingCall.hangUp();  

Nit: trailing whitespace

Comment 3

[Robert Wood [:rwood]]

Thanks Dave. I checked the logcat and it just says X 'should equal' X for is() so I will just leave it as is (pun intended? haha).

Setting for checkin-needed.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5cb7daf9dc

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ff5cb7daf9dc