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

New verb proposal: catkin test #397

Open
mikepurvis opened this issue Aug 24, 2016 · 1 comment
Open

New verb proposal: catkin test #397

mikepurvis opened this issue Aug 24, 2016 · 1 comment

Comments

@mikepurvis
Copy link
Member

mikepurvis commented Aug 24, 2016

I know there's been a bunch of previous discussion about this, and the following aliases exist:

test: build --verbose --make-args test --
run_tests: build --verbose --catkin-make-args run_tests --

However, IMO these are insufficient; a native test verb is required in order to:

  • Do the right thing with a cmake/catkin workspace, which I believe is to run a regular build, and then do make help in each build space to identify whether tests and run_tests targets exist, and if they do, execute them, otherwise skip the package.
  • Properly source the devel space of the package under test prior to running tests. This is critical for packages to find test assets, and I don't believe it happens correctly with the alias.
  • Provide an option to do the run_tests steps all serialized (eg, using the existing installspace mutexing scheme), while still permitting a parallel build of the packages and tests.
  • Place all results into a new test space, similar to the log space.

My proposed implementation of this would be to make new functions create_catkin_test_job/create_cmake_test_job which pass through to create_*_build_job but hitch on the addition steps for testing. So it would be possible to simply catkin test without needing to catkin build; catkin test. (Though another option would be to offer an error message if you try to build without testing first... it would be easy to detect.)

Either way, this would piggy-back off the work in #391 which gives us a mutable env dict to add a second loadtestenv stage before executing tests.

@gavanderhoorn
Copy link
Contributor

I would support this if only for this bit:

Do the right thing with a cmake/catkin workspace, which I believe is to run a regular build [..]

It's a minor thing, but right now you have to remember to first (re)build everything before running tests, as otherwise tests that need to be compiled (or depend on things that need to be compiled) will (obviously) fail.

Afaik, there is no way to express these kind of dependencies in a way that Catkin or CMake can make use of (I asked about it some time ago), so getting the build tool to enforce the work flow would be a nice work-around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants