Skip to content

Revert "launch testing : Wait for topics to publish (#274)"#276

Merged
Blast545 merged 1 commit intomasterfrom
blast545/launch_ros_revert274
Oct 11, 2021
Merged

Revert "launch testing : Wait for topics to publish (#274)"#276
Blast545 merged 1 commit intomasterfrom
blast545/launch_ros_revert274

Conversation

@Blast545
Copy link
Copy Markdown
Contributor

@Blast545 Blast545 commented Oct 7, 2021

This reverts commit bbcc0cc.

@Blast545
Revert "launch testing : Wait for topics to publish (#274)"
d75637c 
This reverts commit bbcc0cc.

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 Blast545 force-pushed the blast545/launch_ros_revert274 branch from adf47c7 to d75637c Compare October 7, 2021 17:32
@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 7, 2021 •
edited
Loading

Windows CI Debug, up to rclpy

  • Windows Build Status

Windows CI Debug+ Revert, up to rclpy

  • Windows Build Status

(Mmm, the error does not reproduce in ci.ros2 testing up to rclpy: silly me, I had to test for launch_ros)

@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 11, 2021

It seems the failures do not happen in builds that do not include all the packages. I would argue that the tests here: are enough evidence that the PR is causing trouble.

Besides that, there is a test failure introduced to the cyclone nightlies:
Fci__nightly-cyclonedds_ubuntu_focal_amd64#323

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Oct 11, 2021 •
edited
Loading

Besides that, there is a test failure introduced to the cyclone nightlies:
Fci__nightly-cyclonedds_ubuntu_focal_amd64#323

That one is almost certainly unrelated; we didn't merge bbcc0cc into Foxy.

@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 11, 2021

@clalancette You are right, I won't take it into account for this discussion.

@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 11, 2021

Windows CI Debug, up to rclpy, ament_package, launch_testing_ros

  • Windows Build Status

Windows CI Debug+ Revert, up to rclpy, ament_package, launch_testing_ros

  • Windows Build Status

@Blast545 Blast545 marked this pull request as ready for review October 11, 2021 19:03
@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 11, 2021

CI Debug, up to rclpy, ament_package, launch_testing_ros

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Oct 11, 2021

Awesome, this is looking better now. @Blast545 Can you run one more set of CI with the CMAKE_BUILD_TYPE blank (our default case) and this PR? Once that comes back green, I think we'll be good to merge this.

clalancette
clalancette approved these changes Oct 11, 2021
View reviewed changes
Copy link
Copy Markdown
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.

@adityapande-1995 FYI

@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 11, 2021

@clalancette Sure!

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

@clalancette clalancette mentioned this pull request Oct 11, 2021
Closed
@Blast545 Blast545 merged commit 06a47b0 into master Oct 11, 2021
@delete-merged-branch delete-merged-branch bot deleted the blast545/launch_ros_revert274 branch October 11, 2021 21:31
@jacobperron
Copy link
Copy Markdown
Member

jacobperron commented Oct 11, 2021

The examples recently added in ros2/examples#324 depend on this feature. I expect tonight's jobs will fail to run the example tests without this.

@adityapande-1995 adityapande-1995 mentioned this pull request Oct 11, 2021
Merged
@sloretz sloretz mentioned this pull request Oct 11, 2021
Merged
sloretz added a commit that referenced this pull request Oct 11, 2021
@sloretz
Revert "Revert "launch testing : Wait for topics to publish (#274)" (#…
29a8bf2 
…276)"

This reverts commit 06a47b0.

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Oct 12, 2021

Yep, they failed. Arg, we should have run --packages-above launch_ros in CI here; sorry about that. @Blast545 can you take a look?

@Blast545
Copy link
Copy Markdown
Contributor Author

Blast545 commented Oct 12, 2021

@clalancette Yes, I'd take a look.

jacobperron added a commit that referenced this pull request Nov 23, 2021
@jacobperron
Revert "Revert "launch testing : Wait for topics to publish (#274)" (#…
2472c7b 
…276)"

This reverts commit 06a47b0.
@jacobperron jacobperron mentioned this pull request Nov 23, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

@jacobperron jacobperron Awaiting requested review from jacobperron

@mjeronimo mjeronimo Awaiting requested review from mjeronimo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Blast545 @clalancette @jacobperron