Closed Bug 808106 Opened 8 years ago Closed 8 years ago

[WebAPI] WebSMS: Develop tests to verify filtering SMS messages by date

Categories

(Core :: DOM: Device Interfaces, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

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

People

(Reporter: rwood, Assigned: rwood)

References

Details

Attachments

(1 file)

Develop new WebAPI (emulator) tests to verify MozSmsFilter.startDate and MozSmsFilter.endDate (via sms.getMessages).
Blocks: 794562
Attached patch Patch for 808106Splinter Review
Attachment #679001 - Flags: feedback?(vyang)
Comment on attachment 679001 [details] [diff] [review]
Patch for 808106

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

Hi Rob, could you merge the two test scripts? They're mostly the same actually.
Attachment #679001 - Flags: feedback?(vyang)
Vicamo,

Thanks for your feedback however I prefer to keep different test cases in separate tests.  This keeps tests simpler, easier to debug failures, and easier to maintain.  Please give me a feedback+ so I can move on to the next step and have the test reviewed.  I need to get these on TBPL ASAP.  Thanks!

Rob
(In reply to Rob Wood [:rwood] from comment #3)
> Please give me a feedback+ so I can move on to the next step and have the test
> reviewed.

I can't. The two files have almost 4/5 duplicated code, only a few lines in getMsgs() are different.
What we need here is bug 805838 being solved. It would allow us to have modules with common code similar to Mochitests. Right now that's not possible. And I think it will improve things a lot.

I did a quick check to the tests and I would prefer if we could have a bit of documentation comments. That would make it easier for everyone to check what's being tested in each method.
Yep, once we can use common modules the amount of code repetition between tests will be greatly reduced.  When that option is available I can revisit some of my tests and streamline them, however for now it is important to get the tests on TBPL ASAP, so moving along to the next step.
Attachment #679001 - Flags: review?(jgriffin)
(In reply to Rob Wood [:rwood] from comment #6)
> It is important to get the tests on TBPL ASAP, so moving along to the next step.

I really don't know why you open new bugs to block a really important one. There are already marionette test cases for SMS, mochitest and xpcshell tests as well. So why do we block on a new, unrevised test case now?
I really don't understand what you mean Vicamo.  Nothing is blocked. The tests are under review. 794562 mentioned above is just a tracking bug for all the related tests.
Attachment #679001 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/35c328e03795
Whiteboard: [automation-needed-in-aurora][automation-needed-in-beta]
https://hg.mozilla.org/mozilla-central/rev/35c328e03795
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.