Closed Bug 1084233 Opened 5 years ago Closed 5 years ago

Add marionette test for icc.unlockCardLock and icc.getCardLock

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S1 (5dec)

People

(Reporter: edgar, Assigned: edgar)

References

Details

Attachments

(4 files, 9 obsolete files)

2.91 KB, patch
hsinyi
: review+
Details | Diff | Splinter Review
7.81 KB, patch
hsinyi
: review+
Details | Diff | Splinter Review
13.58 KB, patch
edgar
: review+
Details | Diff | Splinter Review
5.06 KB, patch
Details | Diff | Splinter Review
Now we only have test cases for icc.setCardLock and icc.getCardLockRetryCount [1], this bug is filed for icc.unlockCardLock and icc.getCardLock. It always good to have more tests. But we need emulator support facility lock first.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/icc/tests/marionette/test_icc_card_lock.js
Assignee: nobody → echen
Attached patch WIP patch (obsolete) — Splinter Review
Blocks: 1087968
Need a rebase for 1083745.
Attachment #8511845 - Attachment is obsolete: true
Move some utility functions into head.js, then we can reuse them in part4.
Attachment #8519842 - Attachment is obsolete: true
Attachment #8511893 - Attachment is obsolete: true
Comment on attachment 8519769 [details] [diff] [review]
Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v3

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

This patch introduces the head.js which provides utility functions for icc, then we can write tests with promise.
And the stk_helper.js [1] and icc_header.js [2] will be removed after all the tests are moved to promise in bug 1087968.

This patch also separates tests for changing pin and getting retry count to two files.

May I have your review, Hsinyi? Thank you.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/icc/tests/marionette/stk_helper.js
[2] http://dxr.mozilla.org/mozilla-central/source/dom/icc/tests/marionette/icc_header.js
Attachment #8519769 - Flags: review?(htsai)
Attachment #8519806 - Flags: review?(htsai)
Attachment #8519848 - Flags: review?(htsai)
Attachment #8519850 - Flags: review?(htsai)
Comment on attachment 8519769 [details] [diff] [review]
Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v3

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

::: dom/icc/tests/marionette/head.js
@@ +164,5 @@
> +
> +  Promise.resolve()
> +    .then(aTestCaseMain)
> +    .then(() => {}, () => {
> +      ok(false, 'promise rejects during test.');

Please also print out |error| in reject callback.

nit: we can simply use |.catch|.

::: dom/icc/tests/marionette/test_icc_card_lock_change_pin.js
@@ +38,5 @@
> +    })
> +    // Test PIN code changes fail.
> +    // The retry count should be decreased by 1.
> +    .then(() => testChangePin(icc, "1111", DEFAULT_PIN, "IncorrectPassword",
> +                              retryCount -1))

nit" space between "-" and 1.
Attachment #8519769 - Flags: review?(htsai) → review+
Attachment #8519806 - Flags: review?(htsai) → review+
Attachment #8519848 - Flags: review?(htsai) → review+
Comment on attachment 8519850 [details] [diff] [review]
Part 4: Add marionette test for unlocking puk, v2

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

Thank you very much for the tests :D
Attachment #8519850 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #14)
> Comment on attachment 8519769 [details] [diff] [review]
> Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v3
> 
> Review of attachment 8519769 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/icc/tests/marionette/head.js
> @@ +164,5 @@
> > +
> > +  Promise.resolve()
> > +    .then(aTestCaseMain)
> > +    .then(() => {}, () => {
> > +      ok(false, 'promise rejects during test.');
> 
> Please also print out |error| in reject callback.
> 
> nit: we can simply use |.catch|.

Okay, will do. Thank you.

> 
> ::: dom/icc/tests/marionette/test_icc_card_lock_change_pin.js
> @@ +38,5 @@
> > +    })
> > +    // Test PIN code changes fail.
> > +    // The retry count should be decreased by 1.
> > +    .then(() => testChangePin(icc, "1111", DEFAULT_PIN, "IncorrectPassword",
> > +                              retryCount -1))
> 
> nit" space between "-" and 1.

Will do.
Address review comment #14:
- print out |error| and use |.catch()|.
- space between "-" and 1.
Attachment #8519769 - Attachment is obsolete: true
Attachment #8528744 - Flags: review+
Comment on attachment 8519850 [details] [diff] [review]
Part 4: Add marionette test for unlocking puk, v2

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

::: dom/icc/tests/marionette/test_icc_card_lock_unlock_puk.js
@@ +83,5 @@
> +      retryCount = aResult.retryCount;
> +      ok(true, "puk retryCount is " + retryCount);
> +    })
> +
> +    // Test unlock PIN code fail.

typo here, should be "PUK". :p
Address comment #18:
- s/PIN/PUK/
Attachment #8519850 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.