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

Split tests into dedicated test packages #455

Conversation

matthew-reynolds
Copy link
Member

@matthew-reynolds matthew-reynolds commented Feb 12, 2020

Addresses #453

This PR attempts to be as minimal as possible. As much as possible, I only moved code, avoiding changing/adding anything else. The main exception is updating and adding all the required dependencies to the new test packages. Note that, in some cases, these dependencies are somewhat excessive (See follow-up 1) - My goal is to simply match deps to what's currently in the tests, cleanup will come later.

Related follow up PRs I'm working on:

  1. Remove unused files from tests. These include several rviz files and "view model" launch files, which don't belong in our test packages and pull in excessive dependencies (rviz, rqt_plot, and in some cases old Hydro-era packages like gazebo and gazebo_worlds)
  2. Cleanup CMakeLists.txt, package.xml, and give a thorough pass at dependencies (Similar to Update CMakeLists.txt and package.xml ros_control#404). I gave a thorough pass at deps for the new test packages, but didn't touch existing packages.

@matthew-reynolds matthew-reynolds changed the title Separate tests Split tests into dedicated test packages Feb 12, 2020
Comment on lines +32 to +33
<!-- <test_depend>gazebo</test_depend> -->
<!-- <test_depend>gazebo_worlds</test_depend> -->
Copy link
Member Author

@matthew-reynolds matthew-reynolds Feb 12, 2020

Choose a reason for hiding this comment

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

Note that these two deps are commented since they have not existed since Hydro, but are used in view_diffbot.launch and view_skidsteerbot.launch.

Once those two files are deleted as part of my follow-up cleanup, these deps will be deleted as well. Same goes for all of the excessive <test_depends> throughout this PR (rqt_plot, rviz, etc.) - All will be deleted once the tests using them are deleted in my follow-up.

@matthew-reynolds
Copy link
Member Author

matthew-reynolds commented Feb 12, 2020

This PR also resolves #378

@bmagyar
Copy link
Member

bmagyar commented Feb 19, 2020

We have recently merged #463 and #457 which should improve the stability of all unit tests. Sorry for the inconvenience of conflicts on this.

@bmagyar
Copy link
Member

bmagyar commented Feb 19, 2020

@ipa-mdl do you have any comments regarding this?

@mathias-luedtke
Copy link
Contributor

I would not split the tests..

Remove unused files from tests.

If I recall it correctly, these "unused" files are meant to be used for local (non-CI) tests.

@bmagyar bmagyar closed this Dec 3, 2020
@matthew-reynolds matthew-reynolds deleted the separate-tests branch December 3, 2020 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants