Closed Bug 848617 Opened 7 years ago Closed 7 years ago

New mochitests for Alarm API

Categories

(Core :: DOM: Core & HTML, defect)

22 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: Luqman, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

Expand the test suite for the Alarm API to better exercise it.
Attached patch Alarm api tests. (obsolete) — Splinter Review
Attachment #721958 - Flags: review?(gene.lian)
Attached patch Alarm api tests. (obsolete) — Splinter Review
Forgot to add new files to patch.
Attachment #721958 - Attachment is obsolete: true
Attachment #721958 - Flags: review?(gene.lian)
Attachment #721959 - Flags: review?(gene.lian)
Comment on attachment 721959 [details] [diff] [review]
Alarm api tests.

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

Hooray! This is super awesome! Thanks Luqman! :)

Btw, I'm wodering how can these mochitests run on the try server? Since it would always fail when attempting to set an alarm in the system which is not a real FFOS mobile device. That is, the code at [1] will return false when adding an alarm, thus throwing an exception and fail all the mochitests you've done. One simple way to avoid that is not to throw an exception for whatever platforms we're testing on. I think it makes sense to do that because we only need to test if we could successfully add/get/remove alarms through the Alarm APIs and don't care about if the alarm will succeed to fire, though. Testing alarm firing needs to take the System Message APIs into considerations, which is another big topic and kind of beyond this issue.

Could you please try to remove the exception? That is, don't need to check the true/false value the |this._alarmHalService.setAlarm(...)| returns. In this way, your mochitests might be able to pass through all kinds of platforms on the try server.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/alarm/AlarmService.jsm#77
Attachment #721959 - Flags: review?(gene.lian) → review+
It would just timeout instead of throwing an exception if I changed AlarmService. Since onsuccess/onerror wouldn't be triggered.
(In reply to Luqman Aden [:Luqman] from comment #4)
> It would just timeout instead of throwing an exception if I changed
> AlarmService. Since onsuccess/onerror wouldn't be triggered.

That's really weird... So you mean in your original mochitests the onsuccess/onerror can be triggered as expected (without exception thrown)?
Oh, I must've misunderstood. Anyways, I've updated the patch a bit so it only actually does the tests on Firefox OS. It works locally when I run the mochitests on my Mac but seems to be timing out on the bots when I push to try. I'll look over it some more tomorrow and will hopefully have it finished off.
Attached patch Alarm api tests.Splinter Review
Finally figured out why the tests were timing out on try. Seems like this needs an emulator update.

try with updated emu: https://tbpl.mozilla.org/?tree=Try&rev=a854800255d1
Attachment #721959 - Attachment is obsolete: true
I believe this can go through now as the new emulator landed in m-i as part of bug 851406.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/284c4adfc17c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.