Closed
Bug 797296
Opened 12 years ago
Closed 12 years ago
B2G STK: write marionette tests for STK proactive commands
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: hsinyi, Assigned: johnshih.bugs)
References
Details
Attachments
(15 files, 47 obsolete files)
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johnshih.bugs
:
review+
|
Details | Diff | Splinter Review |
We should have high-level API tests for STK features, such as sending STK responses and sending STK menu selection. Even more, setting up a call by STK or setting up an event list.
CCing Al
As Bug 809294 supports STK proactive command.
This bug should include following STK features:
- send STK response.
- send STK envelope and envelope with status
- send "REPORT_STK_SERVICE_IS_RUNNGING'
- set/get STK Profile
- send 'session end' from icc
The remaining requests are more complicated
- call setup
- RIL_REQUEST_STK_HANDLE_CALL_SETUP_REQUESTED_FROM_SIM
Comment 2•12 years ago
|
||
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #1)
> As Bug 809294 supports STK proactive command.
wrong bug id?
Updated•12 years ago
|
Updated•12 years ago
|
Assignee: nobody → atsai
Attachment #709568 -
Flags: review?
Attachment #709569 -
Flags: review?
Attachment #709570 -
Flags: review?
Attachment #709571 -
Flags: review?
Attachment #709572 -
Flags: review?
Attachment #709573 -
Flags: review?
Attachment #709574 -
Flags: review?
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #709575 -
Flags: review?
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #709576 -
Flags: review?
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #709578 -
Flags: review?
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #709579 -
Flags: review?
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #709580 -
Flags: review?
Assignee | ||
Comment 16•12 years ago
|
||
Attachment #709581 -
Flags: review?
Assignee | ||
Comment 17•12 years ago
|
||
Attachment #709582 -
Flags: review?
Attachment #709577 -
Flags: review?
Comment on attachment 709568 [details] [diff] [review]
Part 1: Test STK Refresh.
Please update the description for that patch, i.e. "Part 1: Test STK Refresh".
And you should send r? to me, allstars.chh@mozilla.
And in the bug title, please use Capital case,
i.e.
"Bug 797296 - Part 1: Test STK Refresh"
Attachment #709568 -
Attachment description: bug_797296_part1.patch → Part 1: Test STK Refresh.
Comment on attachment 709568 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709568 [details] [diff] [review]:
-----------------------------------------------------------------
I think you also need to update the manifest.ini as well.
::: dom/icc/tests/marionette/test_stk_refresh.js
@@ +7,5 @@
> +
> +let icc = navigator.mozMobileConnection.icc;
> +ok(icc instanceof MozIccManager, "icc is instanceof " + icc.constructor);
> +
> +function testRefresh(cmd, exp) {
Is exp short for 'expect' ?
'exp' is too ambitious please use a better name here.
@@ +16,5 @@
> + runNextTest();
> +}
> +
> +let tests = [
> +
nit, extra line.
@@ +19,5 @@
> +let tests = [
> +
> + {command: "d0108103010101820281829205013f002fe2",
> + func: testRefresh,
> + exp: {name: "pull_off_112",
Why the name is pull off here?
I though this test is for Refresh.
And what does 112 mean here?
@@ +23,5 @@
> + exp: {name: "pull_off_112",
> + commandQualifier: 0x01}},
> + {command: "d009810301010482028182",
> + func: testRefresh,
> + exp: {name: "pull_off_151",
same here.
Attachment #709568 -
Flags: review?
Since John is working on this, assign this bug to him.
Assignee: atsai → jshih
I'll file another bug for other STK features mentioned in comment 1.
Summary: B2G STK: write marionette tests for STK features → B2G STK: write marionette tests for STK proactive commands
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #2)
> (In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #1)
> > As Bug 809294 supports STK proactive command.
>
> wrong bug id?
Sorry I notice this until now, the bug should be 809284.
Blocks: b2g-stk
Attachment #709569 -
Attachment description: bug_797296_part2.patch → Part2: Test STK Poll Off.
Attachment #709569 -
Flags: review? → review?(allstars.chh)
Attachment #709568 -
Flags: review?(allstars.chh)
Attachment #709569 -
Attachment description: Part2: Test STK Poll Off. → Part 2: Test STK Poll Off.
Attachment #709570 -
Attachment description: bug_797296_part3.patch → Part 3: Test STK Setup Event List.
Attachment #709570 -
Flags: review? → review?(allstars.chh)
Attachment #709571 -
Attachment description: bug_797296_part4.patch → Part 4: Test STK Setup Call.
Attachment #709571 -
Flags: review? → review?(allstars.chh)
Attachment #709572 -
Attachment description: bug_797296_part5.patch → Part 5: Test STK Send SS.
Attachment #709572 -
Flags: review? → review?(allstars.chh)
Attachment #709573 -
Attachment description: bug_797296_part6.patch → Part 6: Test STK Send USSD.
Attachment #709573 -
Flags: review? → review?(allstars.chh)
Attachment #709574 -
Attachment description: bug_797296_part7.patch → Part 7: Test STK Send SMS.
Attachment #709574 -
Flags: review? → review?(allstars.chh)
Attachment #709575 -
Attachment description: bug_797296_part8.patch → Part 8: Test STK Send DTMF.
Attachment #709575 -
Flags: review? → review?(allstars.chh)
No longer depends on: 816926
Attachment #709576 -
Attachment description: bug_797296_part9.patch → Part 9: Test STK Launch Browser.
Attachment #709576 -
Flags: review? → review?(allstars.chh)
Attachment #709577 -
Attachment description: bug_797296_part10.patch → Part 10: Test STK Display Text.
Attachment #709577 -
Flags: review? → review?(allstars.chh)
Attachment #709578 -
Attachment description: bug_797296_part11.patch → Part 11: Test STK Get Inkey.
Attachment #709578 -
Flags: review? → review?(allstars.chh)
Attachment #709579 -
Attachment description: bug_797296_part12.patch → Part 12: Test STK Get Input.
Attachment #709579 -
Flags: review? → review?(allstars.chh)
Attachment #709580 -
Attachment description: bug_797296_part13.patch → Part 13: Test STK Select Item.
Attachment #709580 -
Flags: review? → review?(allstars.chh)
Attachment #709581 -
Attachment description: bug_797296_part14.patch → Part 14: Test STK Setup Menu.
Attachment #709581 -
Flags: review? → review?(allstars.chh)
I suggest you finish Part 1 first otherwise you need to do the same thing for all your patches.
Attachment #709582 -
Attachment description: bug_797296_part15.patch → Part 15: Test STK Setup Idle Mode Text.
Attachment #709582 -
Flags: review? → review?(allstars.chh)
Comment on attachment 709568 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709568 [details] [diff] [review]:
-----------------------------------------------------------------
Please address my previous comments first before sending r?
Attachment #709568 -
Flags: review?(allstars.chh) → review-
These patches share some code for sending STK PDU. When Bug 805838 is landed, we could reuse those code instead of repeating it in each test script.
Depends on: 805838
Assignee | ||
Comment 26•12 years ago
|
||
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #19)
> Comment on attachment 709568 [details] [diff] [review]
> Part 1: Test STK Refresh.
>
> Review of attachment 709568 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I think you also need to update the manifest.ini as well.
>
> ::: dom/icc/tests/marionette/test_stk_refresh.js
> @@ +7,5 @@
> > +
> > +let icc = navigator.mozMobileConnection.icc;
> > +ok(icc instanceof MozIccManager, "icc is instanceof " + icc.constructor);
> > +
> > +function testRefresh(cmd, exp) {
>
> Is exp short for 'expect' ?
> 'exp' is too ambitious please use a better name here.
>
> @@ +16,5 @@
> > + runNextTest();
> > +}
> > +
> > +let tests = [
> > +
>
> nit, extra line.
>
> @@ +19,5 @@
> > +let tests = [
> > +
> > + {command: "d0108103010101820281829205013f002fe2",
> > + func: testRefresh,
> > + exp: {name: "pull_off_112",
>
> Why the name is pull off here?
> I though this test is for Refresh.
>
> And what does 112 mean here?
I just followed the naming in the existing test cases (test-stkutil.c)
so I'm not sure what the correct naming/number for each event.
>
> @@ +23,5 @@
> > + exp: {name: "pull_off_112",
> > + commandQualifier: 0x01}},
> > + {command: "d009810301010482028182",
> > + func: testRefresh,
> > + exp: {name: "pull_off_151",
>
> same here.
Assignee | ||
Comment 27•12 years ago
|
||
Attachment #709568 -
Attachment is obsolete: true
Attachment #709626 -
Flags: review?(allstars.chh)
(In reply to John Shih from comment #26)
>
> I just followed the naming in the existing test cases (test-stkutil.c)
> so I'm not sure what the correct naming/number for each event.
>
Seems it's just a name for the test, can you rename it to simpler or meaningful one?
Assignee | ||
Comment 29•12 years ago
|
||
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #28)
> (In reply to John Shih from comment #26)
> >
> > I just followed the naming in the existing test cases (test-stkutil.c)
> > so I'm not sure what the correct naming/number for each event.
> >
>
> Seems it's just a name for the test, can you rename it to simpler or
> meaningful one?
So can I just order by number (e.g, xxx_1. xxx_2, xxx_3 ...) ?
Comment on attachment 709626 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709626 [details] [diff] [review]:
-----------------------------------------------------------------
Please address comment 18 and comment 19 first before sending r?
This is for the second time I said this.
Attachment #709626 -
Flags: review?(allstars.chh) → review-
(In reply to John Shih from comment #29)
> So can I just order by number (e.g, xxx_1. xxx_2, xxx_3 ...) ?
Sound okay.
Please do this.
Comment on attachment 709626 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709626 [details] [diff] [review]:
-----------------------------------------------------------------
Please address comment 18 and comment 19 first before sending r?
This is for the second time I said this.
::: dom/icc/tests/marionette/test_stk_refresh.js
@@ +10,5 @@
> +
> +function testRefresh(cmd, expect) {
> + log("STK CMD " + JSON.stringify(cmd));
> + is(cmd.typeOfCommand, icc.STK_CMD_REFRESH, expect.name);
> + is(cmd.commandQualifier, exp.commandQualifier, expect.name);
TEST-UNEXPECTED-FAIL | test_stk_refresh.js | JavascriptException: ReferenceError: exp is not defined.
Please make sure you run the tests before sending r?
Assignee | ||
Comment 33•12 years ago
|
||
I've changed the naming to xxx_cmd_# and fixed the typo
but sadly I cannot pass this case
got
raise ScriptTimeoutException(message=message, status=status, stacktrace=stacktrace)
TEST-UNEXPECTED-FAIL | test_stk_refresh.js | ScriptTimeoutException: timed out
Attachment #709626 -
Attachment is obsolete: true
Attachment #709630 -
Attachment description: Test STK Refresh. → Part 1: Test STK Refresh.
Assignee | ||
Comment 34•12 years ago
|
||
Attachment #709569 -
Attachment is obsolete: true
Attachment #709569 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 35•12 years ago
|
||
Comment on attachment 709634 [details] [diff] [review]
Part 2: Test STK Poll Off.
Since I can't pass STK Refresh, please review this one first
thanks
Attachment #709634 -
Flags: review?(allstars.chh)
Comment on attachment 709630 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709630 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_refresh.js
@@ +19,5 @@
> +let tests = [
> + {command: "d0108103010101820281829205013f002fe2",
> + func: testRefresh,
> + expect: {name: "refresh_cmd_1",
> + commandQualifier: 0x01}},
nit, align this line.
@@ +23,5 @@
> + commandQualifier: 0x01}},
> + {command: "d009810301010482028182",
> + func: testRefresh,
> + expect: {name: "refresh_cmd_2",
> + commandQualifier: 0x04}}
ditto
Attachment #709634 -
Flags: review?(allstars.chh) → review?(allstars.chh)
Comment on attachment 709634 [details] [diff] [review]
Part 2: Test STK Poll Off.
Review of attachment 709634 [details] [diff] [review]:
-----------------------------------------------------------------
This patch is much better now.
Please update bug comment, "Bug 797296", not "bug 797296"
There's a rule for this.
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities
Please address nits below, and add r=me.
thanks
::: dom/icc/tests/marionette/test_stk_poll_off.js
@@ +7,5 @@
> +
> +let icc = navigator.mozMobileConnection.icc;
> +ok(icc instanceof MozIccManager, "icc is instanceof " + icc.constructor);
> +
> +function testPollOff(cmd, expect) {
For consistency, please rename cmd to command,
and all occurrences below.
@@ +19,5 @@
> +let tests = [
> + {command: "d009810301040082028182",
> + func: testPollOff,
> + expect: {name: "pull_off_cmd_1",
> + commandQualifier: 0x00}}
nit, align this.
Attachment #709634 -
Flags: review?(allstars.chh) → review+
Comment on attachment 709630 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 709630 [details] [diff] [review]:
-----------------------------------------------------------------
Please update patch comment and add r=me.
::: dom/icc/tests/marionette/test_stk_refresh.js
@@ +7,5 @@
> +
> +let icc = navigator.mozMobileConnection.icc;
> +ok(icc instanceof MozIccManager, "icc is instanceof " + icc.constructor);
> +
> +function testRefresh(cmd, expect) {
For consistency please rename cmd to command, and all occurrences below.
Attachment #709630 -
Flags: review+
Assignee | ||
Comment 39•12 years ago
|
||
update
Attachment #709630 -
Attachment is obsolete: true
Attachment #711184 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 40•12 years ago
|
||
update
Attachment #709634 -
Attachment is obsolete: true
Attachment #711187 -
Flags: review?(allstars.chh)
Comment on attachment 711184 [details] [diff] [review]
Part 1: Test STK Refresh.
Review of attachment 711184 [details] [diff] [review]:
-----------------------------------------------------------------
I already gave you r+ so you don't have to r? again.
BTW, my bugzilla email is allstars.chh@mozilla.com, gmail.com is my personal mail
Attachment #711184 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 42•12 years ago
|
||
update. (with test pass)
Attachment #709570 -
Attachment is obsolete: true
Attachment #709570 -
Flags: review?(allstars.chh)
Attachment #711190 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 43•12 years ago
|
||
update (with test pass)
Attachment #709571 -
Attachment is obsolete: true
Attachment #709571 -
Flags: review?(allstars.chh)
Attachment #711192 -
Flags: review?(allstars.chh)
Attachment #711187 -
Flags: review?(allstars.chh) → review+
Attachment #711190 -
Flags: review?(allstars.chh) → review?(allstars.chh)
Comment on attachment 711190 [details] [diff] [review]
Part 3: Test STK Setup Event List.
Review of attachment 711190 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_event_list.js
@@ +11,5 @@
> +function testSetupEventList(command, expect) {
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SET_UP_EVENT_LIST, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> +
We should also test command.options.eventList here
@@ +39,5 @@
> + commandQualifier: 0x00}},
> + {command: "d00c810301050082028182990107",
> + func: testSetupEventList,
> + expect: {name: "setup_event_list_cmd_6",
> + commandQualifier: 0x00}}
nit, trailing ws.
Attachment #711190 -
Flags: review?(allstars.chh)
Comment on attachment 711192 [details] [diff] [review]
Part 4: Test STK Setup Call.
Review of attachment 711192 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_call.js
@@ +11,5 @@
> +function testSetupCall(command, expect) {
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SET_UP_CALL, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + if(typeof command.options.confirmMessage != "undefined") {
if (command.options.confirmMessage) seems enough.
@@ +82,5 @@
> + {command: "d04c81030110008202818385165365742075702063616c6c2049636f6e20332e342e318609911032042143651c2c9e02000185165365742075702063616c6c2049636f6e20332e342e329e020001",
> + func: testSetupCall,
> + expect: {name: "setup_call_cmd_13",
> + commandQualifier: 0x00,
> + text: "Set up call Icon 3.4.1"}},
nit, trailing space.
@@ +112,5 @@
> + {command: "d02c810301100082028183850e434f4e4649524d4154494f4e20328609911032042143651c2c850643414c4c2032",
> + func: testSetupCall,
> + expect: {name: "setup_call_cmd_19",
> + commandQualifier: 0x00,
> + text: "CONFIRMATION 2"}},
ditto.
@@ +242,5 @@
> + {command: "d02081030110008202818385058030eb003186079110320421436585058030eb0032",
> + func: testSetupCall,
> + expect: {name: "setup_call_cmd_45",
> + commandQualifier: 0x00,
> + text: "ル1"}},
ditto.
Attachment #711192 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 48•12 years ago
|
||
Attachment #711209 -
Attachment is obsolete: true
Attachment #711210 -
Flags: review+
Comment on attachment 709572 [details] [diff] [review]
Part 5: Test STK Send SS.
Review of attachment 709572 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_send_ss.js
@@ +58,5 @@
> + title: "Basic Icon"}},
> + {command: "d01d810301110082028183890e91aa120a214365870921436587b99e020101",
> + func: testSendSS,
> + exp: {name: "send_ss_241",
> + commandQualifier: 0x00}},
This test doesn't have title,
however title is required in SEND_SS.
Can you remove this test?
@@ +153,5 @@
> + {command: "d02d810301110082028183851054657874204174747269627574652033891091aa120a214365870921436587a901fb",
> + func: testSendSS,
> + exp: {name: "send_ss_473",
> + commandQualifier: 0x00,
> + title: "Toolkit Select 3"}},
The title seems wrong here.
Comment on attachment 709573 [details] [diff] [review]
Part 6: Test STK Send USSD.
Review of attachment 709573 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_send_ussd.js
@@ +43,5 @@
> + title: "once a RELEASE COMPLETE message containing the USSD Return Result message not containing an error has been received from the network, the ME shall inform the SIM that the command has"}},
> + {command: "d0448103011200820281838a39f041e19058341e9149e592d9743ea151e9945ab55eb1596d2b2c1e93cbe6333aad5eb3dbee373c2e9fd3ebf63b3eaf6fc564335acd76c3e560",
> + func: testSendUSSD,
> + exp: {name: "send_ussd_171",
> + commandQualifier: 0x00}},
No title.
@@ +67,5 @@
> + title: "Basic Icon"}},
> + {command: "d0488103011200820281838a39f041e19058341e9149e592d9743ea151e9945ab55eb1596d2b2c1e93cbe6333aad5eb3dbee373c2e9fd3ebf63b3eaf6fc564335acd76c3e5609e020101",
> + func: testSendUSSD,
> + exp: {name: "send_ussd_241",
> + commandQualifier: 0x00}},
No title here.
@@ +152,5 @@
> + {command: "d05c8103011200820281838510546578742041747472696275746520318a39f041e19058341e9149e592d9743ea151e9945ab55eb1596d2b2c1e93cbe6333aad5eb3dbee373c2e9fd3ebf63b3eaf6fc564335acd76c3e560d004001020b4",
> + func: testSendUSSD,
> + exp: {name: "send_ussd_471",
> + commandQualifier: 0x00,
> + title: "Toolkit Select 1"}},
The title seems wrong here.
Assignee | ||
Comment 51•12 years ago
|
||
add check of command.options.eventList
Attachment #711190 -
Attachment is obsolete: true
Assignee | ||
Comment 53•12 years ago
|
||
update
Assignee | ||
Comment 54•12 years ago
|
||
update (with test pass)
Attachment #709572 -
Attachment is obsolete: true
Attachment #711646 -
Attachment is obsolete: true
Attachment #709572 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 55•12 years ago
|
||
update (with test pass)
Attachment #709573 -
Attachment is obsolete: true
Attachment #709573 -
Flags: review?(allstars.chh)
Attachment #711647 -
Flags: review?(allstars.chh)
Attachment #711648 -
Flags: review?(allstars.chh)
Attachment #711650 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 57•12 years ago
|
||
update (with test pass)
Attachment #709574 -
Attachment is obsolete: true
Attachment #709574 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 58•12 years ago
|
||
Attachment #711662 -
Attachment is obsolete: true
Assignee | ||
Comment 59•12 years ago
|
||
Attachment #711650 -
Attachment is obsolete: true
Attachment #711650 -
Flags: review?(allstars.chh)
Attachment #711665 -
Flags: review?(allstars.chh)
Attachment #711663 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 60•12 years ago
|
||
update (with test pass)
Attachment #709575 -
Attachment is obsolete: true
Attachment #709575 -
Flags: review?(allstars.chh)
Attachment #711669 -
Flags: review?(allstars.chh)
Comment on attachment 711647 [details] [diff] [review]
Part 3: Test STK Setup Event List.
Review of attachment 711647 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_event_list.js
@@ +28,5 @@
> + {command: "d00d81030105008202818299020507",
> + func: testSetupEventList,
> + expect: {name: "setup_event_list_cmd_2",
> + commandQualifier: 0x00,
> + eventList: 5}},
eventList is an array,
and in this case the array has two elements [5, 7],
so the test in testSetupEventLust should test with the contents of eventList.
Attachment #711647 -
Flags: review?(allstars.chh)
Attachment #711648 -
Flags: review?(allstars.chh) → review+
Attachment #711663 -
Flags: review?(allstars.chh) → review+
Attachment #711665 -
Flags: review?(allstars.chh) → review+
Attachment #711669 -
Flags: review?(allstars.chh) → review+
Comment on attachment 709576 [details] [diff] [review]
Part 9: Test STK Launch Browser.
Review of attachment 709576 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_launch_browser.js
@@ +7,5 @@
> +
> +let icc = navigator.mozMobileConnection.icc;
> +ok(icc instanceof MozIccManager, "icc is instanceof " + icc.constructor);
> +
> +function testLaunchBrowser(cmd, exp) {
please rename these parameters.
@@ +17,5 @@
> + runNextTest();
> +}
> +
> +let tests = [
> +
nit, spaces.
also many of them below.
Attachment #709576 -
Flags: review?(allstars.chh)
Comment on attachment 709576 [details] [diff] [review]
Part 9: Test STK Launch Browser.
Review of attachment 709576 [details] [diff] [review]:
-----------------------------------------------------------------
what about manifest.ini?
Comment on attachment 709577 [details] [diff] [review]
Part 10: Test STK Display Text.
Review of attachment 709577 [details] [diff] [review]:
-----------------------------------------------------------------
Same problem here.
Attachment #709577 -
Flags: review?(allstars.chh)
Comment on attachment 709578 [details] [diff] [review]
Part 11: Test STK Get Inkey.
Review of attachment 709578 [details] [diff] [review]:
-----------------------------------------------------------------
Same
Attachment #709578 -
Flags: review?(allstars.chh)
Attachment #709579 -
Flags: review?(allstars.chh)
Attachment #709580 -
Flags: review?(allstars.chh)
Attachment #709581 -
Flags: review?(allstars.chh)
Attachment #709582 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 69•12 years ago
|
||
Attachment #711700 -
Attachment is obsolete: true
Attachment #711702 -
Flags: review?(allstars.chh)
Attachment #711705 -
Flags: review?(allstars.chh)
Attachment #711706 -
Flags: review?(allstars.chh)
Attachment #711709 -
Flags: review?(allstars.chh)
Attachment #711702 -
Flags: review?(allstars.chh) → review+
Attachment #711705 -
Flags: review?(allstars.chh) → review+
Comment on attachment 711706 [details] [diff] [review]
Part 9: Test STK Launch Browser
Review of attachment 711706 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_launch_browser.js
@@ +14,5 @@
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + if (command.options.confirmMessage) {
> + is(command.options.confirmMessage, expect.text, expect.name);
> + }
> +
Can you also the command.options.url?
Attachment #711706 -
Flags: review?(allstars.chh)
Attachment #711714 -
Flags: review?(allstars.chh)
Attachment #711718 -
Flags: review?(allstars.chh)
Attachment #711722 -
Flags: review?(allstars.chh)
Comment on attachment 711705 [details] [diff] [review]
Part 11: Test STK Get Inkey.
Review of attachment 711705 [details] [diff] [review]:
-----------------------------------------------------------------
Cancel the r+, as I found out we'd test more properties here.
::: dom/icc/tests/marionette/test_stk_get_inkey.js
@@ +12,5 @@
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_GET_INKEY, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + is(command.options.text, expect.text, expect.name);
> +
For GET_INKEY
we should also check command.options.{isAlphabet, isUCS2, and isYesNoRequired};
Attachment #711705 -
Flags: review+
Comment on attachment 711709 [details] [diff] [review]
Part 12: Test STK Get Input.
Review of attachment 711709 [details] [diff] [review]:
-----------------------------------------------------------------
The same with GET_INKEY, we should test those isAlpha, isUCS2,.. properties.
(Those properties are extracted from Command Qualifier).
Attachment #711709 -
Flags: review?(allstars.chh)
Comment on attachment 711714 [details] [diff] [review]
Part 13: Test STK Select Item.
Review of attachment 711714 [details] [diff] [review]:
-----------------------------------------------------------------
Can we also test item[] ?
Attachment #711714 -
Flags: review?(allstars.chh)
Comment on attachment 711718 [details] [diff] [review]
Part 14: Test STK Setup Menu.
Review of attachment 711718 [details] [diff] [review]:
-----------------------------------------------------------------
Same with SELECT_ITEM
Attachment #711718 -
Flags: review?(allstars.chh)
Comment on attachment 711722 [details] [diff] [review]
Part 15: Test STK Setup Idle Mode Text.
Review of attachment 711722 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_idle_mode_text.js
@@ +12,5 @@
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SET_UP_IDLE_MODE_TEXT, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + is(command.options.text, expect.text, expect.name);
> +
Can we also test isHighPriority, userClear and responseNeeded?
Attachment #711722 -
Flags: review?(allstars.chh)
Comment on attachment 711702 [details] [diff] [review]
Part 10: Test STK Display Text.
Review of attachment 711702 [details] [diff] [review]:
-----------------------------------------------------------------
Cancel the r+ as we'd also test other properties in DISPLAY_TEXT.
::: dom/icc/tests/marionette/test_stk_display_text.js
@@ +12,5 @@
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_DISPLAY_TEXT, expect.name);
> + is(command.options.text, expect.text, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> +
We should also test isHighPriority, userClear and responseNeed here.
Attachment #711702 -
Flags: review+
Assignee | ||
Comment 81•12 years ago
|
||
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #79)
> Comment on attachment 711722 [details] [diff] [review]
> Part 15: Test STK Setup Idle Mode Text.
>
> Review of attachment 711722 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: dom/icc/tests/marionette/test_stk_setup_idle_mode_text.js
> @@ +12,5 @@
> > + log("STK CMD " + JSON.stringify(command));
> > + is(command.typeOfCommand, icc.STK_CMD_SET_UP_IDLE_MODE_TEXT, expect.name);
> > + is(command.commandQualifier, expect.commandQualifier, expect.name);
> > + is(command.options.text, expect.text, expect.name);
> > +
>
> Can we also test isHighPriority, userClear and responseNeeded?
I got response in the form below:
INFO STK CMD {"commandNumber":1,"typeOfCommand":40,"commandQualifier":0,"rilMessageType":"stkcommand","options":{"text":"Idle Mode Text 2"}} Fri Feb 08 2013 08:17:13 GMT+0000 (GMT)
how can I get the fields you mention?
Assignee | ||
Comment 82•12 years ago
|
||
add the check of userClear, isHighPriority
Attachment #711702 -
Attachment is obsolete: true
Attachment #711734 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 83•12 years ago
|
||
add check of url
Attachment #711706 -
Attachment is obsolete: true
Attachment #711738 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 84•12 years ago
|
||
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #61)
> Comment on attachment 711647 [details] [diff] [review]
> Part 3: Test STK Setup Event List.
>
> Review of attachment 711647 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: dom/icc/tests/marionette/test_stk_setup_event_list.js
> @@ +28,5 @@
> > + {command: "d00d81030105008202818299020507",
> > + func: testSetupEventList,
> > + expect: {name: "setup_event_list_cmd_2",
> > + commandQualifier: 0x00,
> > + eventList: 5}},
>
> eventList is an array,
> and in this case the array has two elements [5, 7],
> so the test in testSetupEventLust should test with the contents of eventList.
Can you provide a proper way to test the contents of array? (I wonder if for loop is valid for test cases)
or can I just compare their length?
thanks
Attachment #711722 -
Flags: review+
Attachment #711734 -
Flags: review?(allstars.chh) → review+
Attachment #711738 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 85•12 years ago
|
||
Check array instead of single element
Attachment #711647 -
Attachment is obsolete: true
Attachment #715331 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 86•12 years ago
|
||
add check of isAlphabet, isUSC2, isYesNoRequested.
Attachment #711705 -
Attachment is obsolete: true
Assignee | ||
Comment 87•12 years ago
|
||
add check of isAlphabet, isUCS2, isPacked, hideInput.
Attachment #711709 -
Attachment is obsolete: true
Comment on attachment 715331 [details] [diff] [review]
Part 3: Test STK Setup Event List.
Review of attachment 715331 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_event_list.js
@@ +11,5 @@
> +function testSetupEventList(command, expect) {
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SET_UP_EVENT_LIST, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + for(index in command.options.eventList) {
nit, space after for
Attachment #715331 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 89•12 years ago
|
||
add check of items[].
Attachment #711714 -
Attachment is obsolete: true
Assignee | ||
Comment 90•12 years ago
|
||
add check of items[].
Attachment #711718 -
Attachment is obsolete: true
Attachment #715333 -
Flags: review?(allstars.chh)
Attachment #715335 -
Flags: review?(allstars.chh)
Attachment #715344 -
Flags: review?(allstars.chh)
Attachment #715352 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 91•12 years ago
|
||
fix nit
Attachment #715331 -
Attachment is obsolete: true
Attachment #715354 -
Flags: review+
Comment on attachment 715333 [details] [diff] [review]
Part 11: Test STK Get Inkey.
Review of attachment 715333 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_get_inkey.js
@@ +20,5 @@
> + is(command.options.isUCS2, expect.isUCS2, expect.name);
> + }
> + if (command.options.isYesNoRequested) {
> + is(command.options.isYesNoRequested, expect.isYesNoRequested, expect.name);
> + }
nit, add an extra line before runNextTest
Attachment #715333 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 93•12 years ago
|
||
fix nit
Attachment #711210 -
Attachment is obsolete: true
Attachment #715356 -
Flags: review+
Attachment #715335 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 94•12 years ago
|
||
fix nit.
Attachment #711648 -
Attachment is obsolete: true
Attachment #715359 -
Flags: review+
Assignee | ||
Comment 95•12 years ago
|
||
fix nit.
Attachment #711665 -
Attachment is obsolete: true
Attachment #715361 -
Flags: review+
Comment on attachment 715344 [details] [diff] [review]
Part 13: Test STK Select Item.
Review of attachment 715344 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_select_item.js
@@ +12,5 @@
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SELECT_ITEM, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + is(command.options.title, expect.title, expect.name);
> + for (index in command.options.items) {
let index ..
Attachment #715344 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 98•12 years ago
|
||
fix nit.
Attachment #711669 -
Attachment is obsolete: true
Attachment #715364 -
Flags: review+
Comment on attachment 715352 [details] [diff] [review]
Part 14: Test STK Setup Menu.
Review of attachment 715352 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_setup_menu.js
@@ +12,5 @@
> + log("STK CMD " + JSON.stringify(command));
> + is(command.typeOfCommand, icc.STK_CMD_SET_UP_MENU, expect.name);
> + is(command.commandQualifier, expect.commandQualifier, expect.name);
> + is(command.options.title, expect.title, expect.name);
> + for (index in command.options.items) {
let index
Attachment #715352 -
Flags: review?(allstars.chh) → review+
Hi John,
Could you run try server on B2G platform?
thanks
Assignee | ||
Comment 101•12 years ago
|
||
update.
Attachment #715354 -
Attachment is obsolete: true
Attachment #715365 -
Flags: review+
Assignee | ||
Comment 102•12 years ago
|
||
update.
Attachment #715333 -
Attachment is obsolete: true
Attachment #715367 -
Flags: review+
Assignee | ||
Comment 103•12 years ago
|
||
Attachment #715367 -
Attachment is obsolete: true
Attachment #715369 -
Flags: review+
Assignee | ||
Comment 104•12 years ago
|
||
update.
Attachment #715344 -
Attachment is obsolete: true
Attachment #715371 -
Flags: review+
Assignee | ||
Comment 105•12 years ago
|
||
update.
Attachment #715352 -
Attachment is obsolete: true
Attachment #715374 -
Flags: review+
Assignee | ||
Comment 106•12 years ago
|
||
Attachment #711663 -
Attachment is obsolete: true
Attachment #715362 -
Attachment is obsolete: true
Attachment #715377 -
Flags: review+
Assignee | ||
Comment 107•12 years ago
|
||
I've tested the patches on try server,
please see
https://tbpl.mozilla.org/?tree=Try&rev=87f8b4b8c0fc
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jshih@mozilla.com-87f8b4b8c0fc/try-linux/
Cool, I'll help to push them when the test is finished,
also next time you'd run try only on B2G platform.
Otherwise sheriff would complain.
Import script for common code I filed Bug 843455 to address it.
Assignee | ||
Comment 110•12 years ago
|
||
little error on previous test,
so I push another test. (rename the part 15 patch to correct one)
https://tbpl.mozilla.org/?tree=Try&rev=59f25c918d58
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jshih@mozilla.com-59f25c918d58
John, your 'try' result is failed, although I don't know what went wrong,
Can you run it again on 'B2G only' platform?
Assignee | ||
Comment 112•12 years ago
|
||
ok, I'll check it and do another test
Assignee | ||
Comment 113•12 years ago
|
||
New test run.
https://tbpl.mozilla.org/?tree=Try&rev=15684a551bbc
CCing Griffin on this,
Hi, Griffin
In the Part 4 patch, there are some Chinese and Japanese characters in that test, we ran it well on our local PCs, but when we run try server, it fails with error message "UnicodeEncodeError: 'ascii' codec can't encode character u'\u30eb' in position 131: ordinal not in range(128)", (see comment 113)
Is this a known issue for marionette ?
Depends on: 847256
Filed Bug 847256 for the TBPL failure,
John Can you comment out those failed tests and add comments on it?
Like
/*
TBPL UnicodeEncodeError, See Bug 847256.
{command:...
func:...
expect:...
}
*/
thanks
Hi, John
Can you try the patch from Bug 847256 with your patches and run the try server again?
thanks
Assignee | ||
Comment 117•12 years ago
|
||
Hi,
the test is here.
https://tbpl.mozilla.org/?tree=Try&rev=7c2034aa9aa5
Please also apply patch from Bug 847683 for the uploadssymbols error.
Comment 119•12 years ago
|
||
Try run for acedca11d51c is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=acedca11d51c
Results (out of 1 total builds):
failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/yhuang@mozilla.com-acedca11d51c
Assignee | ||
Comment 120•12 years ago
|
||
The patch in Bug 847683 seems strange..
I can't find the file "b2g_config.py" in the path "mozilla/"
This makes the import fail
Comment 121•12 years ago
|
||
Try run for b6aa786f5bbb is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=b6aa786f5bbb
Results (out of 1 total builds):
failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/yhuang@mozilla.com-b6aa786f5bbb
Assignee | ||
Comment 122•12 years ago
|
||
Hi, I've push a test again and seems all pass
please see
https://tbpl.mozilla.org/?tree=Try&rev=42ea5b7953b6
p.s. there is a fail unrelated to my patch
Hi, John
Thanks.
Once Bug 847256 is landed I'll commit your patches.
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=0b9505f9af26
Thanks for your hard work, John.
Comment 125•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/df8f39f4e927
https://hg.mozilla.org/mozilla-central/rev/a999b78a26bd
https://hg.mozilla.org/mozilla-central/rev/deef21121046
https://hg.mozilla.org/mozilla-central/rev/85a99a931072
https://hg.mozilla.org/mozilla-central/rev/60240a6d40e9
https://hg.mozilla.org/mozilla-central/rev/322bfcca74c2
https://hg.mozilla.org/mozilla-central/rev/23e438b0264c
https://hg.mozilla.org/mozilla-central/rev/42424b62ca23
https://hg.mozilla.org/mozilla-central/rev/d75651cff1be
https://hg.mozilla.org/mozilla-central/rev/7ce9fa20fb19
https://hg.mozilla.org/mozilla-central/rev/3b9b3ba143eb
https://hg.mozilla.org/mozilla-central/rev/29707091ab83
https://hg.mozilla.org/mozilla-central/rev/2f6ef14fa3b5
https://hg.mozilla.org/mozilla-central/rev/35d864904312
https://hg.mozilla.org/mozilla-central/rev/0b9505f9af26
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•