Closed Bug 819213 Opened 8 years ago Closed 8 years ago

Re-enable test_filter_number_single/multiple.js marionette tests

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: gwagner, Assigned: rwood)

References

Details

Attachments

(1 file, 1 obsolete file)

I had to disable these sms marionette tests because I had no way to debug them (bug 818833).
rwood offered to help here.
Blocks: 811538
Maybe vicamo can help as well.
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
Flags: needinfo?(anygregor)
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.
Flags: needinfo?(anygregor)
Thanks Gregor, I will update the filter tests to use the international number version.
Assignee: nobody → rwood
Attached patch Updated SMS number filter tests (obsolete) — Splinter Review
Use international number format and re-enable the tests on TBPL. Ran the tests locally and they pass.
Attachment #691974 - Flags: review?(anygregor)
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
Attachment #691974 - Flags: review?(anygregor) → review+
> 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!).
Attachment #691974 - Attachment is obsolete: true
Attachment #691997 - Flags: review?(anygregor)
Attachment #691997 - Flags: review?(anygregor) → review+
Keywords: checkin-needed
Whiteboard: [automation-needed-in-aurora], [automation-needed-in-b2g18]
https://hg.mozilla.org/mozilla-central/rev/8ef4348286d5
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.