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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect, P4)

Description

[Philipp von Weitershausen [:philikon]]

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.

Comment 1

[Philipp von Weitershausen [:philikon]]

When/if we manage to write these tests, we can also remove the test_between_emulator.py test.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Why can't we write the tests as pure JS?  We can keep around "cached" emulators to make requesting them cheaper.

Comment 3

[Andrew Overholt [:overholt]]

Nomming for basecamp as per https://bugzilla.mozilla.org/show_bug.cgi?id=709564#c1.

Comment 4

[Philipp von Weitershausen [:philikon]]

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

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

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

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

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

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github Pull Request for external/qemu

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko test cases

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

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.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

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

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko test cases

Re-request for review after bug 814761 and bug 820220 being landed.

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

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.

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

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.

Comment 14

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko test cases

Add one more test case for verifying gsm 7bit string decoding in emulator.

Comment 15

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

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

Please see my comments in github :)

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

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

https://github.com/mozilla-b2g/platform_external_qemu/pull/10 , commit 3f0a512 reviewed.

Comment 17

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

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);
}

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko test cases

Address review comment

Comment 19

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

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

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko test cases

Update nits. Trying whether emulator has been updated: https://tbpl.mozilla.org/?tree=Try&rev=e48ccb6c9fa8

Comment 21

[Mozilla RelEng Bot]

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/[e-mail]-e48ccb6c9fa8

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

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

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

try after bug 830252: https://tbpl.mozilla.org/?tree=Try&rev=0be96c7d81d7

Comment 24

[Mozilla RelEng Bot]

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/[e-mail]-0be96c7d81d7

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b0765410460

Comment 26

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/0b0765410460