Closed Bug 897117 Opened 8 years ago Closed 8 years ago

[Clock] ClockView methods are not fully covered by unit tests

Categories

(Firefox OS Graveyard :: Gaia::Clock, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jugglinmike, Assigned: jugglinmike)

References

Details

Attachments

(1 file, 1 obsolete file)

Not all ClockView methods have associated unit tests.
Attached patch Bug-897117-clockview-tests.patch (obsolete) — Splinter Review
Hey Ian,

As noted in the commit message, this patch also fixes a bug in ClockView:

> This commit also fixes a bug in the tested method's implementation,
> where the minute boundary was being calculated incorrectly. This bug
> would induce the method to asynchronously recurse during the second of
> time before the minute rolled over.

I've also submitted this work as a pull request on GitHub: https://github.com/mozilla-b2g/gaia/pull/11130
Attachment #779869 - Flags: review?(iliu)
Comment on attachment 779869 [details] [diff] [review]
Bug-897117-clockview-tests.patch

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

Hi Mike,

The logic of the additional test is right. But the expected value is not equal in different timezone notation.
I have left some comment on Github. Thanks for your effort.
Attachment #779869 - Flags: review?(iliu) → review-
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Blocks: 898666
I've abstracted the tests to work regardless of system timezone. I've left my work as separate commits, but I will squash prior to landing in `master`.
Attachment #779869 - Attachment is obsolete: true
Attachment #782767 - Flags: review?(iliu)
Comment on attachment 782767 [details] [diff] [review]
Bug-897117-clockview-tests-v2.patch

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

The solution is okay for me. It must works on different servers.
Attachment #782767 - Flags: review?(iliu) → review+
Squashed and landed on `master` at commit 19339bbf483c318df7a7ba749aca605d6d6fc2f1

Thanks for the review, Ian! This was a tricky one :)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.