Closed Bug 1087968 Opened 5 years ago Closed 5 years ago

Rewrite icc marionette tests with Promise

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S5 (6feb)

People

(Reporter: edgar, Assigned: edgar)

References

Details

Attachments

(4 files, 3 obsolete files)

No description provided.
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
We could reuse the head.js introduced in bug 1084233.
Depends on: 1084233
Depends on: 945576
No longer depends on: 945576
Comment on attachment 8551817 [details] [diff] [review]
Part 1: Split test_stk_proactive_command.js into localInfo and timerManagement, v1

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

Split test_stk_proactive_command.js into two tests, test_stk_local_info.js and test_stk_timer_management.js. And also rewrite them with Promise.
Hi Hsinyi, would you mind reviewing this? Thank you.
Attachment #8551817 - Flags: review?(htsai)
Attachment #8552182 - Flags: review?(htsai)
Attachment #8552917 - Flags: review?(htsai)
Comment on attachment 8552926 [details] [diff] [review]
Part 4: Deferred object is obsoleted since Gecko 30, use new Promise instead, v1

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

Deferred in Promise.jsm is obsoleted since Gecko 30 [1], we should use new Promise instead.
Although we don't use Promise.jsm in icc marionette tests, but I would still like to remove faked Deferred and follow the standard way to create a new promise.

Hi Hsinyi, may I have your review? Thank you.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Promise.jsm/Deferred
Attachment #8552926 - Flags: review?(htsai)
Comment on attachment 8551817 [details] [diff] [review]
Part 1: Split test_stk_proactive_command.js into localInfo and timerManagement, v1

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

Looks good thought it would be more perfect that |sendEmulatorStkPdu| is added in part 1 instead of part 2 ;)
Attachment #8551817 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #9)
> Comment on attachment 8551817 [details] [diff] [review]
> Part 1: Split test_stk_proactive_command.js into localInfo and
> timerManagement, v1
> 
> Review of attachment 8551817 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good thought it would be more perfect that |sendEmulatorStkPdu| is
> added in part 1 instead of part 2 ;)

Oops!! Will move |sendEmulatorStkPdu| to part 1, thanks for catching this.
Attachment #8552182 - Flags: review?(htsai) → review+
Comment on attachment 8552917 [details] [diff] [review]
Part 3: Rewrite test_icc_* with Promise, v2

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

Looks good!
Attachment #8552917 - Flags: review?(htsai) → review+
Attachment #8552926 - Flags: review?(htsai) → review+
Assignee: nobody → echen
Address review comment #9
- Moving |sendEmulatorStkPdu| to part 1
Attachment #8551817 - Attachment is obsolete: true
Attachment #8554349 - Flags: review+
Address review comment #9
- Moving |sendEmulatorStkPdu| to part 1
Attachment #8552182 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.