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

Status: RESOLVED FIXED

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

Description

[Mike Pennisi [:jugglinmike]]

Not all ClockView methods have associated unit tests.

Comment 1

[Mike Pennisi [:jugglinmike]]

Attachment: Bug-897117-clockview-tests.patch

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

Comment 2

[[e-mail], [e-mail]]

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.

Comment 3

[Mike Pennisi [:jugglinmike]]

Attachment: Bug-897117-clockview-tests-v2.patch

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`.

Comment 4

[[e-mail], [e-mail]]

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.

Comment 5

[Mike Pennisi [:jugglinmike]]

Squashed and landed on `master` at commit 19339bbf483c318df7a7ba749aca605d6d6fc2f1

Thanks for the review, Ian! This was a tricky one :)