1084233 - Add marionette test for icc.unlockCardLock and icc.getCardLock

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Edgar Chen [:edgar]]

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

Comment 1

[Edgar Chen [:edgar]]

Attachment: WIP patch

Comment 2

[Edgar Chen [:edgar]]

Attachment: Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v1

Comment 3

[Edgar Chen [:edgar]]

Attachment: Part 2: Add marionette test for enabling/disabling pin lock, v1

Comment 4

[Edgar Chen [:edgar]]

Attachment: Part 3: Add marionette test for unlocking pin, v1

Comment 5

[Edgar Chen [:edgar]]

Attachment: Part 4: Add marionette test for unlocking puk, v1

Comment 6

[Edgar Chen [:edgar]]

Attachment: Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v2

Comment 7

[Edgar Chen [:edgar]]

Need a rebase for 1083745.

Comment 8

[Edgar Chen [:edgar]]

Attachment: Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v3

Comment 9

[Edgar Chen [:edgar]]

Attachment: Part 2: Add marionette test for enabling/disabling pin lock, v2

Comment 10

[Edgar Chen [:edgar]]

Attachment: Part 3: Add marionette test for unlocking pin, v2

Comment 11

[Edgar Chen [:edgar]]

Attachment: Part 3: Add marionette test for unlocking pin, v3

Move some utility functions into head.js, then we can reuse them in part4.

Comment 12

[Edgar Chen [:edgar]]

Attachment: Part 4: Add marionette test for unlocking puk, v2

Comment 13

[Edgar Chen [:edgar]]

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

Comment 14

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

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.

Comment 15

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

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

Comment 16

[Edgar Chen [:edgar]]

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

Comment 17

[Edgar Chen [:edgar]]

Attachment: Part 1: Introduce head.js and rewrite test_icc_card_lock.js with Promise, v4

Address review comment #14:
- print out |error| and use |.catch()|.
- space between "-" and 1.

Comment 18

[Edgar Chen [:edgar]]

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

Comment 19

[Edgar Chen [:edgar]]

Attachment: Part 4: Add marionette test for unlocking puk, v3

Address comment #18:
- s/PIN/PUK/

Comment 20

[Edgar Chen [:edgar]]

Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=ed69ea52ca32

Comment 21

[Edgar Chen [:edgar]]

https://hg.mozilla.org/integration/b2g-inbound/rev/3d6c6dd443ae
https://hg.mozilla.org/integration/b2g-inbound/rev/3392f55163d3
https://hg.mozilla.org/integration/b2g-inbound/rev/d638e47fe638
https://hg.mozilla.org/integration/b2g-inbound/rev/1ce89b6f8989

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d6c6dd443ae
https://hg.mozilla.org/mozilla-central/rev/3392f55163d3
https://hg.mozilla.org/mozilla-central/rev/d638e47fe638
https://hg.mozilla.org/mozilla-central/rev/1ce89b6f8989