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.

Sign up for GitHub

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

Jump to bottom

Add pytest for descriptions #342

Merged
merged 6 commits into from
Dec 25, 2023
Merged

Add pytest for descriptions #342

merged 6 commits into from
Dec 25, 2023

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Aug 14, 2023 •
edited
Loading

I added some simple tests to this repo.

I copied them over from https://github.com/UniversalRobots/Universal_Robots_ROS2_Description

Locally, colcon test does not run it, why? I had to manually install pip install -U pytest, otherwise an old version got installed causing import errors.

@christophfroehlich christophfroehlich marked this pull request as ready for review August 17, 2023 06:41
destogl
destogl previously approved these changes Sep 18, 2023
View reviewed changes
example_1/test/test_view_robot_launch.py Show resolved Hide resolved
@destogl
Copy link
Member

destogl commented Sep 18, 2023

Can we actually rather add integration tests? This file might be an interesting inspiration:

https://github.com/StoglRobotics-forks/kuka_experimental/pull/12/files#diff-b2da8ae937165518fe9c1d9f7fc39094e9c0fd179eb1389115ca444936f06a3c

@christophfroehlich
Copy link
Contributor Author

Can we actually rather add integration tests? This file might be an interesting inspiration:

https://github.com/StoglRobotics-forks/kuka_experimental/pull/12/files#diff-b2da8ae937165518fe9c1d9f7fc39094e9c0fd179eb1389115ca444936f06a3c

This PR is the first step, just testing if xacro is happy with the urdf and if the view_robot.launch.py is starting without any errors.
I'll have a look, thanks for the link.

@christophfroehlich christophfroehlich changed the title Add pytest for example1 description Add pytest for descriptions Dec 25, 2023
@christophfroehlich
Copy link
Contributor Author

@Mergifyio backport iron humble

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 25, 2023 •
edited
Loading

backport iron humble

✅ Backports have been created

saikishor
saikishor previously approved these changes Dec 25, 2023
View reviewed changes
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Good work. LTGM
Thank you

saikishor
saikishor approved these changes Dec 25, 2023
View reviewed changes
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the recent changes

@christophfroehlich christophfroehlich merged commit eba962d into ros-controls:master Dec 25, 2023
12 checks passed
mergify bot pushed a commit that referenced this pull request Dec 25, 2023
@christophfroehlich @mergify
Add pytest for example1 description (#342)
8efd0b1 
(cherry picked from commit eba962d)
mergify bot pushed a commit that referenced this pull request Dec 25, 2023
@christophfroehlich @mergify
Add pytest for example1 description (#342)
e8827df 
(cherry picked from commit eba962d)
This was referenced Dec 25, 2023
Merged
Merged
saikishor pushed a commit that referenced this pull request Dec 25, 2023
@mergify @christophfroehlich
Add pytest for example1 description (#342) (#418)
f501bb6 
(cherry picked from commit eba962d)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
saikishor pushed a commit that referenced this pull request Dec 25, 2023
@mergify @christophfroehlich
Add pytest for example1 description (#342) (#419)
c393709 
(cherry picked from commit eba962d)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
@christophfroehlich christophfroehlich mentioned this pull request Dec 25, 2023
Open
@christophfroehlich christophfroehlich deleted the add_simple_tests branch December 25, 2023 19:09
olivier-stasse
olivier-stasse reviewed Dec 26, 2023
View reviewed changes
Copy link
Collaborator

@olivier-stasse olivier-stasse left a comment

Choose a reason for hiding this comment

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

Dear @christophfroehlich thanks for the PR.
I believe that the license is not correct.

example_9/test/test_view_robot_launch.py
@@ -0,0 +1,54 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dear @christophfroehlich thanks a lot for this very nice addition.
However I believe that all the licenses part is a copy pase that should be removed.
First it is a BSD Clause 2 license, while APACHE is the license claimed in the root license file.
Second the author is somebody else...

As this repository is for people learning ros2_control and that unit test is very important, removing this unnecessary part will help to keep the example clear.

Again thanks a lot for this welcome addition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint, I'm not sure about the license stuff and how ament_copyright deals with it.. But I can try ;) You mean that I just remove the license part but let the line with the copyright claim?

example_6/test/test_view_robot_launch.py
@@ -0,0 +1,54 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove license, just add instition and author name, and refer to the license in the repository's root.

example_7/test/test_urdf_xacro.py
@@ -0,0 +1,77 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove license, just add instition and author name, and refer to the license in the repository's root.

example_7/test/test_view_robot_launch.py
@@ -0,0 +1,54 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove license, just add instition and author name, and refer to the license in the repository's root.

example_8/test/test_urdf_xacro.py
@@ -0,0 +1,77 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove license, just add instition and author name, and refer to the license in the repository's root.

example_8/test/test_view_robot_launch.py
@@ -0,0 +1,54 @@
# Copyright (c) 2022 FZI Forschungszentrum Informatik
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove license, just add instition and author name, and refer to the license in the repository's root.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olivier-stasse olivier-stasse olivier-stasse left review comments

@saikishor saikishor saikishor approved these changes

@destogl destogl destogl left review comments

@bmagyar bmagyar Awaiting requested review from bmagyar

@jaron-l jaron-l Awaiting requested review from jaron-l

@LukasMacha97 LukasMacha97 Awaiting requested review from LukasMacha97

@mcbed mcbed Awaiting requested review from mcbed

@progtologist progtologist Awaiting requested review from progtologist

@VanshGehlot VanshGehlot Awaiting requested review from VanshGehlot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@christophfroehlich @destogl @olivier-stasse @saikishor