Merged
Conversation
…ge.xml and enhance rosbag data handling
…date tests and remove unused code
until they release the new ROS2 support
There was a problem hiding this comment.
Copilot reviewed 215 out of 226 changed files in this pull request and generated no comments.
Files not reviewed (11)
- .env.sample: Language not supported
- Dockerfile: Language not supported
- deps.repos: Language not supported
- docker/Dockerfile: Language not supported
- docker/build.bash: Language not supported
- docker/entrypoint.sh: Language not supported
- docker/join.bash: Language not supported
- docker/run.bash: Language not supported
- rerun/.gitignore: Language not supported
- rerun/requirements.txt: Language not supported
- src/nav2_gz_testing/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (1)
src/nav2_gz_testing/launch/waypoint_follower_example_launch.py:87
- The command arguments include a nested list (["headless:=", headless]) which may not be interpreted correctly. Consider concatenating 'headless:=' with the value of headless or flattening the list before passing to ExecuteProcess.
world_sdf_xacro = ExecuteProcess( cmd=["xacro", "-o", world_sdf, ["headless:=", headless], world] )
tomonorman
reviewed
Apr 14, 2025
tomonorman
approved these changes
Apr 14, 2025
Collaborator
tomonorman
left a comment
There was a problem hiding this comment.
Thank you, very cool.
So I think some issues / todos we could create for the cli / toolkit could be:
- Have a.
pytestoption in artefacts.yaml. (Instead ofrunthat is used in this branch) - The rosbag upload logic should apply to
frameworknotros_testfileif that is the case - Cleanup of output_dirs (default=true?) with flag or config
get_artefacts_paramshould have a default fallback (eg if not using artefacts)- If there are any pytest fixtures / functions that might be "commonly" used perhaps we should make a
artefacts_toolkit.pytestmodule
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade to Jazzy
Main changes:
pytestwith ros2launch_pytestutils to run the tests instead oflaunch_testlaunch_test: https://github.com/ros2/launch/tree/rolling/launch_pytest#differences-with-launch_testingUnfortunately, almost all files were changed/moved/removed so reviewing the diff will be probably difficult. Might make more sense to take a look at the source as it is: https://github.com/art-e-fact/navigation2_ignition_gazebo_example/tree/jazzy-harmonic-migration
Dashboard: https://app.artefacts.com/artefacts/navigation2-ignition-example
nav2_gz.mp4