Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix flaky test #1063

Merged
merged 6 commits into from
Apr 17, 2023
Merged

fix flaky test #1063

merged 6 commits into from
Apr 17, 2023

Conversation

iuhilnehc-ynos
Copy link
Collaborator

@iuhilnehc-ynos iuhilnehc-ynos commented Apr 17, 2023

to fix #1062

void expect_topics_types(

is only called in VerifySubsystemCount, which expect to cost (20*400ms) if failure happened in the test,
but we can find [ FAILED ] NodeGraphMultiNodeFixture.test_node_info_publishers (3909 ms) just consume 4s running the test.

I think this ASSERT can be removed as the discovery in operations (1, 2, 3, 4) might be asynchronous(doc needs to be updated) which is similar to rmw_get_publishers_info_by_topic.

rcl_get_publisher_names_and_types_by_node -> rmw_get_publisher_names_and_types_by_node -> rmw_api_connextdds_get_publisher_names_and_types_by_node -> ... -> get_writer_names_and_types_by_node -> GraphCache::get_writer_names_and_types_by_node ->
https://github.com/ros2/rmw_dds_common/blob/0dc8af3cc1c471c60a96e896ab66633d486feb50/rmw_dds_common/src/graph_cache.cpp#L891-L898

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos
Copy link
Collaborator Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Chen Lihui and others added 5 commits April 17, 2023 14:48
Co-authored-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
This reverts commit a1f3c0d.

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
This reverts commit 1c83504.

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI.

@clalancette
Copy link
Contributor

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

OK, this one looks good to me. @fujitatomoya can you take another look, and if you agree approve and merge this one? Thanks.

@clalancette clalancette added this to In progress in Iron Irwini via automation Apr 17, 2023
@fujitatomoya
Copy link
Collaborator

@clalancette thanks!

@fujitatomoya fujitatomoya merged commit a1ba916 into ros2:rolling Apr 17, 2023
Iron Irwini automation moved this from In progress to Done Apr 17, 2023
danthony06 pushed a commit to danthony06/rcl that referenced this pull request Jun 14, 2023
* fix flaky test

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

* avoid RCL_RET_NODE_NAME_NON_EXISTENT case instead of removing ASSERT

Co-authored-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>

* return immediately

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

* set the out parameter

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

* Revert "set the out parameter"

This reverts commit a1f3c0d.

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

* Revert "return immediately"

This reverts commit 1c83504.

Signed-off-by: Chen Lihui <lihui.chen@sony.com>

---------

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Co-authored-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Test test_graph__rmw_connextdds failing on nightly_win_rep
3 participants