Skip to content

add test for timers#25

Merged
dirk-thomas merged 1 commit intomasterfrom
timer_test
Aug 4, 2015
Merged

add test for timers#25
dirk-thomas merged 1 commit intomasterfrom
timer_test

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Aug 3, 2015
@dirk-thomas dirk-thomas self-assigned this Aug 3, 2015
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Aug 3, 2015

You'll have to use spin or spin_once to test what I'm talking about. Spin some will result in a non-blocking spin, so timers could be handled when rmw_wait wakes up. You need a blocking call to rmw_wait or a timed call to rmw_wait in order to test what I was describing. That's why I suggested calling spin in a thread so it can block.

@dirk-thomas
Copy link
Copy Markdown
Member Author

I have added the test you described in 51d1565988c26fc718b2d3de0e21ca2aa1497e4b.

http://ci.ros2.org/job/ros2_batch_ci_linux/134/
http://ci.ros2.org/job/ros2_batch_ci_osx/66/
http://ci.ros2.org/job/ros2_batch_ci_windows/93/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using the mechanism you're testing in order to shutdown the test is not a reliable way to make sure the test stops. In fact if this test fails as I expect it to, this callback will not be executed, the test will hang, and the test runner will have to kill it.

@dirk-thomas
Copy link
Copy Markdown
Member Author

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Aug 3, 2015

lgtm, thanks for iterating on it.

@dirk-thomas
Copy link
Copy Markdown
Member Author

With the increased timeout (9c2114d96ba6893dcf3b4180ea6d085fa6bd6c38) the tests pass:
http://ci.ros2.org/job/ros2_batch_ci_linux/136/
http://ci.ros2.org/job/ros2_batch_ci_osx/68/

I have also created ament/ament_cmake#19 to better handle this case.

dirk-thomas added a commit that referenced this pull request Aug 4, 2015
@dirk-thomas dirk-thomas merged commit 27132fb into master Aug 4, 2015
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Aug 4, 2015
@dirk-thomas dirk-thomas deleted the timer_test branch August 4, 2015 05: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.

2 participants