You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've noticed that once in a while test_graph_empty in graph.rs will have a failed assertion here where rclrs only detects 5 services instead of the expected 6.
I believe this is most likely a race condition in discovery where most of the time service discovery is finished by the time we query for the services, but that's not guaranteed because the underlying rmw implementation is usually multi-threaded.
In #421 I introduce a notify_on_graph_change function which can be used in conjunction with until_promise_resolved to spin until the node graph satisfies some condition. I recommend that we use this combo to keep spinning until the number of services reaches the expected value. That should eliminate all flakiness, assuming this really is just a multi-threaded discovery problem.
I'm opening this issue just to remind myself to fix this test once #421 is merged. If a maintainer can assign the issue to me, I would appreciate it.
The text was updated successfully, but these errors were encountered:
I've noticed that once in a while
test_graph_empty
ingraph.rs
will have a failed assertion here where rclrs only detects 5 services instead of the expected 6.I believe this is most likely a race condition in discovery where most of the time service discovery is finished by the time we query for the services, but that's not guaranteed because the underlying rmw implementation is usually multi-threaded.
In #421 I introduce a
notify_on_graph_change
function which can be used in conjunction withuntil_promise_resolved
to spin until the node graph satisfies some condition. I recommend that we use this combo to keep spinning until the number of services reaches the expected value. That should eliminate all flakiness, assuming this really is just a multi-threaded discovery problem.I'm opening this issue just to remind myself to fix this test once #421 is merged. If a maintainer can assign the issue to me, I would appreciate it.
The text was updated successfully, but these errors were encountered: