Make test_executor.spin_some_max_duration more reliable#430
Make test_executor.spin_some_max_duration more reliable#430
test_executor.spin_some_max_duration more reliable#430Conversation
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
clalancette
left a comment
There was a problem hiding this comment.
One small change to add a bit of documentation, but I'll approve anyway.
| auto node = rclcpp::Node::make_shared("spin_some_max_duration"); | ||
| auto lambda = []() { | ||
| std::this_thread::sleep_for(1ms); | ||
| std::this_thread::sleep_for(100ms); |
There was a problem hiding this comment.
This is a good change, and keeps the current semantics of the test; basically, that 20 threads of N ms sleep take < 20*N ms to complete. However, those semantics are a bit hard to discern here. Would you mind adding a comment explaining that?
There was a problem hiding this comment.
There was already a comment, but I forgot to update it.
See 2d0e91c.
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
CI, testing only
|
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
The test seemed to be "less" flaky in the above run, but it still failed after a bunch of repetitions.
Based on internet speculations (as windows doesn't officially clarify anything), it seems that windows server distributions have a bigger time slice that desktop versions. That's (maybe) why time based tests started failing more frequently when we switched to containerized builds. |
|
@clalancette let me know if you think this is ready. |
clalancette
left a comment
There was a problem hiding this comment.
@clalancette let me know if you think this is ready.
I can further increase time tolerance to 500ms, as timing isn't really the point of the test.
Yeah, I think that would be a good idea. As long as it completes in less than 2 seconds, I think we've proven what the test is trying to prove. I'll still approve.
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
CI:
|
Fixes https://ci.ros2.org/view/nightly/job/nightly_win_rel/1537/testReport/junit/test_rclcpp/test_executor__rmw_cyclonedds_cpp/spin_some_max_duration/.
Considering that the Windows scheduler time slice is 20ms, the test is being too optimistic (same applies for macOS and Linux, though it was only failing on Windows).
I increased all durations, to ensure that the OS time slice doesn't represent a big fraction of it.