819213 - Re-enable test_filter_number_single/multiple.js marionette tests

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gregor Wagner [:gwagner]]

I had to disable these sms marionette tests because I had no way to debug them (bug 818833).
rwood offered to help here.

Comment 1

[Gregor Wagner [:gwagner]]

Maybe vicamo can help as well.

Comment 2

[Robert Wood [:rwood]]

Hi Gregor,

test_filter_number_single/multiple.js are still failing even with a new build. If I add "+1" to the front of the number that is used for the actual MozSMS Filter, then getMessages/the filter will find the expected number of SMS messages (whether or not the original SMS sender actually has the "+1" or not, but if it is in the filter then the SMS is found).

Is it a requirement now that when specifying an SMS number to filter messages by, you must add the "+1" to the start of the filter number?  If so, I will modify the tests to use that new filter format, and will re-enable them on TBPL. Or, is this a bug?  Should the filter handle filtering numbers without the "+1" at the start?

Thanks!

Rob

Comment 3

[Gregor Wagner [:gwagner]]

We convert all local numbers to the international format once we store it in the sms DB. So it should start with "+1" and it depends on the country you are currently in.

The remaining question is how this behaves when the tests are run outside of the US or Canada. The international version depends on the country you are in. 
Maybe it would be best to just use international versions in the test here so that we avoid the conversion in our DB code.

Comment 4

[Robert Wood [:rwood]]

Thanks Gregor, I will update the filter tests to use the international number version.

Comment 5

[Robert Wood [:rwood]]

Attachment: Updated SMS number filter tests

Use international number format and re-enable the tests on TBPL. Ran the tests locally and they pass.

Comment 6

[Gregor Wagner [:gwagner]]

Comment on attachment 691974 [details] [diff] [review]
Updated SMS number filter tests

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

I think you can also revert my attempt from https://hg.mozilla.org/mozilla-central/rev/06da32434ea1

Comment 7

[Robert Wood [:rwood]]

Attachment: New patch for 819213

> I think you can also revert my attempt from https://hg.mozilla.org/mozilla-central/rev/06da32434ea1

No, the changes you made in the link above also include fixes to test_getmessage.js which is still required, so that push shouldn't be reverted because if the changes to test_getmessage.js are reverted that test would start to fail again, correct?

I have remade this patch with my same changes and also removed your previous changes in test_filter_number_single.js (perhaps that is what you meant anyway, thanks for pointing that out!).

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ef4348286d5

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8ef4348286d5

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1f02f40898ef
https://hg.mozilla.org/releases/mozilla-b2g18/rev/dff6845fc58e