Skip to content

Conversation

@lakshmi-kannan
Copy link
Contributor

No description provided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing license header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a6c5301

@Kami
Copy link
Member

Kami commented Dec 18, 2014

It would also be good to test it "end to "end. Meaning starting a timer and making sure it actually dispatches a trigger.

Besides that, LGTM, +1

@lakshmi-kannan
Copy link
Contributor Author

Added an end to end case in a6c5301. Minimum resolution of timer seems to be 1s.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be on the safe side, I would also add self.assertFalse(mock_dispatcher.assert_payload()) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grrr. Fixed. fe5332b

@Kami
Copy link
Member

Kami commented Dec 18, 2014

Besides that one comment, LGTM, +1

lakshmi-kannan added a commit that referenced this pull request Dec 18, 2014
@lakshmi-kannan lakshmi-kannan merged commit 17cb032 into timer_should_not_be_sensor Dec 18, 2014
@lakshmi-kannan lakshmi-kannan deleted the tests_for_timer branch December 18, 2014 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants