B2G SMS tests: figure out if we can test outgoing SMS without using 2 emulators

RESOLVED FIXED in mozilla21

Status

()

defect
P4
normal
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: philikon, Assigned: vicamo)

Tracking

Trunk
mozilla21
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(2 attachments, 4 obsolete attachments)

We can test sending and receiving SMS with two emulators which has a few disadvantages:

* can't easily isolate sending failures from receiving failures
* booting up a second emulator adds to the total test run
* can't write these tests as pure JS atm

So it'd be nice if we could test outgoing SMS with just 1 emu.
Blocks: 756607
When/if we manage to write these tests, we can also remove the test_between_emulator.py test.
Why can't we write the tests as pure JS?  We can keep around "cached" emulators to make requesting them cheaper.
Nomming for basecamp as per https://bugzilla.mozilla.org/show_bug.cgi?id=709564#c1.
blocking-basecamp: --- → ?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Why can't we write the tests as pure JS?

Because the test harness that drives everything is written in Python. It does support JS-only tests and these can manipulate their own emulator now, but they can't bring up a second emulator (yet).

> We can keep around "cached" emulators to make requesting them cheaper.

We are already doing that. Still means that the code-compile-test cycle is pretty long.

In any case, this is a developer ergonomics bug. Shouldn't be a blocker.
blocking-basecamp: ? → -

Updated

7 years ago
No longer blocks: b2g-sms
Assignee

Comment 5

7 years ago
In external/qemu/telephony/android_modem.c, function handleSendSMSText() seems to loop back SMS if the destination address points to itself.
Assignee

Comment 6

7 years ago
Not so lucky:

  emulator: sms_receiver_add_submit_pdu: created SMS index 1, from 5554, ref 0, max 1
  emulator: sms_receiver_add_submit_pdu: SMS index 1, received all 1 fragments
  remote_call_generic: trying to call self
  handleSendSMSText: could not send SMS PDU to remote emulator
Assignee: nobody → vyang
Assignee

Comment 7

7 years ago
Attachment #686452 - Flags: review?(htsai)
Assignee

Comment 8

7 years ago
Posted patch Gecko test cases (obsolete) — Splinter Review
Attachment #686473 - Flags: review?(htsai)
Assignee

Updated

7 years ago
Blocks: 816082
Assignee

Comment 9

7 years ago
Comment on attachment 686452 [details]
Github Pull Request for external/qemu

Cancel review because Android Emulator UCS2 SMS handling is broken. Need more fixes and test cases.
Attachment #686452 - Flags: review?(htsai)
Assignee

Updated

7 years ago
Attachment #686473 - Flags: review?(htsai)
Assignee

Comment 10

7 years ago
Depends on bug 814761 because we'll have the same problem when testing emulator loopback with multipart messages.
Depends on: 814761
Assignee

Updated

7 years ago
Depends on: 809553
Assignee

Updated

7 years ago
No longer depends on: 809553
Assignee

Updated

7 years ago
Depends on: 820220
Assignee

Comment 11

7 years ago
Posted patch Gecko test cases (obsolete) — Splinter Review
Re-request for review after bug 814761 and bug 820220 being landed.
Attachment #686473 - Attachment is obsolete: true
Attachment #690864 - Flags: review?(htsai)
Assignee

Updated

7 years ago
Attachment #686452 - Flags: review?(htsai)
Assignee

Comment 12

7 years ago
BB? for blocking bug 816082, which is BB+. This bug blocks bug 816082 because we have to verify the sent messages in both local and remote ends. We can also rewrite the test case in python, but fixing this bug first will be more convenient for future development.
blocking-basecamp: - → ?
Review in triage leaves this bug in the basecamp- state. This bug is a strong want and has been marked as basecamp- P4 to reflect this. I have also broken the dependency on bug 816082 as this does not look to actually block.
No longer blocks: 816082
blocking-basecamp: ? → -
Priority: -- → P4
Assignee

Comment 14

7 years ago
Posted patch Gecko test cases (obsolete) — Splinter Review
Add one more test case for verifying gsm 7bit string decoding in emulator.
Attachment #690864 - Attachment is obsolete: true
Attachment #690864 - Flags: review?(htsai)
Attachment #691247 - Flags: review?(htsai)
Comment on attachment 686452 [details]
Github Pull Request for external/qemu

Please see my comments in github :)
Attachment #686452 - Flags: review?(htsai)
Assignee

Comment 16

7 years ago
Comment on attachment 686452 [details]
Github Pull Request for external/qemu

https://github.com/mozilla-b2g/platform_external_qemu/pull/10 , commit 3f0a512 reviewed.
Attachment #686452 - Flags: review+
Comment on attachment 691247 [details] [diff] [review]
Gecko test cases

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

::: dom/sms/tests/marionette/test_emulator_loopback.js
@@ +20,5 @@
> +function randomString16() {
> +  return Math.random().toString(36).substr(2, 16);
> +}
> +
> +function times(str, n) {

I think we can simply implement this function as

function times(str, num) {
  return (new Array(num+1)).join(str);
}
Attachment #691247 - Flags: review?(htsai)
Assignee

Comment 18

7 years ago
Posted patch Gecko test cases (obsolete) — Splinter Review
Address review comment
Attachment #691247 - Attachment is obsolete: true
Attachment #695562 - Flags: review?(htsai)
Comment on attachment 695562 [details] [diff] [review]
Gecko test cases

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

r+ but please be sure to address the typo, thanks!

::: dom/sms/tests/marionette/test_emulator_loopback.js
@@ +22,5 @@
> +}
> +
> +function times(str, n) {
> +  return (new Array(num+1)).join(str);
> +}

typo: s/n/num
Attachment #695562 - Flags: review?(htsai) → review+
Assignee

Updated

7 years ago
Depends on: 824572
Assignee

Comment 20

7 years ago
Update nits. Trying whether emulator has been updated: https://tbpl.mozilla.org/?tree=Try&rev=e48ccb6c9fa8
Attachment #695562 - Attachment is obsolete: true
Attachment #696963 - Flags: review+
Try run for e48ccb6c9fa8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e48ccb6c9fa8
Results (out of 22 total builds):
    success: 21
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/vyang@mozilla.com-e48ccb6c9fa8
Assignee

Comment 22

7 years ago
(In reply to Mozilla RelEng Bot from comment #21)
> Try run for e48ccb6c9fa8 is complete.
>     https://tbpl.mozilla.org/?tree=Try&rev=e48ccb6c9fa8

Another run in https://tbpl.mozilla.org/?tree=Try&rev=d6c44e1fd4d6 completed successfully with no change (but debug flags) made. Trying two more times.
Assignee

Updated

7 years ago
Depends on: 830252
Try run for 0be96c7d81d7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0be96c7d81d7
Results (out of 1 total builds):
    success: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/vyang@mozilla.com-0be96c7d81d7
https://hg.mozilla.org/mozilla-central/rev/0b0765410460
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Assignee

Updated

6 years ago
Duplicate of this bug: 809553
You need to log in before you can comment on or make changes to this bug.