Closed Bug 1100200 Opened 7 years ago Closed 7 years ago

[MarionetteTest] Organize all telephony tests

Categories

(Firefox OS Graveyard :: RIL, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S2 (19dec)

People

(Reporter: aknow, Assigned: aknow)

References

Details

Attachments

(5 files, 1 obsolete file)

1. Migrate all test cases to promise version and utilize head.js
2. Sort the list in manifest.ini
3. Re-examine all the disabled tests and try to enable them.
Depends on: 1100211
Depends on: 1093014
Depends on: 1098193
No longer depends on: 1093014
Sorry for the huge patch.....
Attachment #8523706 - Attachment is obsolete: true
Attachment #8524409 - Flags: review?(htsai)
Comment on attachment 8524409 [details] [diff] [review]
Part 1: Migrate to promise version

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

r=me with 
1) the comment for test_incoming_already_connected.js addressed,
2) have patches for another part to clean up test_emergency.js, test_outgoing_radio_off.js and test_outgoing_error_state.js

Details in-line, thank you!

::: dom/telephony/test/marionette/head.js
@@ +1067,5 @@
>    this.gDelay = delay;
>    this.gWaitForEvent = waitForEvent;
> +  this.gWaitForCallsChangedEvent = waitForCallsChangedEvent;
> +  this.gWaitForNamedStateEvent = waitForNamedStateEvent;
> +  this.gWaitForStateChangeEvent = waitForStateChangeEvent;

These three are added in bug 1093014. Rebase needed.

::: dom/telephony/test/marionette/test_emergency.js
@@ +11,5 @@
> +    .then(call => outCall = call)
> +    .then(() => gRemoteAnswer(outCall))
> +    .then(() => gHangUp(outCall))
> +    .catch(error => ok(false, "Promise reject: " + error))
> +    .then(finish);

Let's have another part for merging this and part of test_outgoing_error_state.js. 

Please add one more case for |gDialEmergency("A Bad Number")| to test_emergency.js.

::: dom/telephony/test/marionette/test_emergency_label.js
@@ +9,5 @@
>  function setEccListProperty(list) {
>    log("Set property ril.ecclist: " + list);
>  
> +  // We should wrap empty |list| by ''. Otherwise, the entire command will be
> +  // "setprop ril.ecclist" which causus the command error.

Fixed in bug 1099053. Rebase needed.

::: dom/telephony/test/marionette/test_incoming_already_connected.js
@@ +27,5 @@
> +
> +    // Answer incoming call; original outgoing call should be held
> +    .then(() => gAnswer(inCall))
> +    .then(() => gCheckAll(inCall, [outCall, inCall], "", [],
> +                          [outInfo.held, inInfo.active]))

Be careful of potential timing issues b/w incall.active and outcall.held. Let's do what has been done in bug 1093014, thank you.

::: dom/telephony/test/marionette/test_outgoing_error_state.js
@@ +22,1 @@
>      .then(() => gDialEmergency(number))

Please have another part for the comment:
Let's move this case to test_emergency.js, and expand test_outgoing_radio_off.js to cover |gDialEmergency(a_normal_number) when radio off|. Then, it's fine to remove the whole test_outgoing_error_state.js.
Attachment #8524409 - Flags: review?(htsai) → review+
Comment on attachment 8524410 [details] [diff] [review]
Part 2: Merge test cases into test_incoming_basic_operations.js

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

r=me with comment addressed.

::: dom/telephony/test/marionette/manifest.ini
@@ +31,1 @@
>  [test_incoming_onstatechange.js]

Don't forget to add " test_incoming_basic_operations.js " to manifest.ini.
Attachment #8524410 - Flags: review?(htsai) → review+
Attachment #8524411 - Flags: review?(htsai) → review+
Comment on attachment 8524412 [details] [diff] [review]
Part 4: Merge test cases into test_outgoing_basic_operations.js

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

Truly nice!

r=me with comments addressed, thank you~

::: dom/telephony/test/marionette/manifest.ini
@@ +43,2 @@
>  [test_outgoing_onstatechange.js]
>  [test_outgoing_radio_off.js]

Don't forget |test_outgoing_basic_operations.js |.

::: dom/telephony/test/marionette/test_incoming_basic_operations.js
@@ +36,5 @@
> +
> +function remoteHangUp() {
> +  return gRemoteHangUp(inCall)
> +    .then(() => gCheckAll(null, [], "", [], []));
> +}

We don't really need this, right :)
Attachment #8524412 - Flags: review?(htsai) → review+
Attachment #8534154 - Flags: review?(htsai) → review+
You need to log in before you can comment on or make changes to this bug.