Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add catkin test verb #676

Merged
merged 29 commits into from
Oct 3, 2021
Merged

Add catkin test verb #676

merged 29 commits into from
Oct 3, 2021

Conversation

timonegk
Copy link
Member

@timonegk timonegk commented Apr 17, 2021

This pull request adds the long-awaited catkin test verb.

Summary of the changes

The test verb aims to replace the current aliases for test and run_tests.
Currently, calling catkin test does the following:

  • For each of the requested packages, check if it is a catkin or a cmake package
  • If it is a catkin package, execute make run_tests in the package
  • If it is a plain cmake package, execute make test in the package
  • Then, call catkin_test_results for the results
    The tests are executed in parallel, that can be prevented with -p 1.
    The test output is shown after all the tests of a package are completed, that can be changed with -i.

Things to do before the merge

  • add some tests
  • test on plain cmake packages
  • handle cmake packages that don't expose a test make target
  • make it possible to pass other test targets than run_tests and test
  • add shell completion
  • update documentation
  • fix --verbose
  • call make cmake_check_build_system before running tests
  • make test output directory configurable
  • avoid filtering too much output on python packages
  • remove the manually incremented version
  • add --make-args

Test it

Please test the new verb on your setup and report any problems or requests in this thread! You can install this version with the command pip3 install git+https://github.com/catkin/catkin_tools.git@catkin_test. If the installation was successful, catkin --version should give the version 0.8.0.

@timonegk timonegk mentioned this pull request Apr 17, 2021
@mikepurvis
Copy link
Member

mikepurvis commented Apr 17, 2021

Nice work— seems reasonable to me. I tried some time ago but attempted that more ambitious path of trying to reach inside packages and then run individual test targets under catkin's parallelism:

https://github.com/mikepurvis/catkin_tools_test/blob/master/catkin_tools_test/test.py

Unsurprisingly, this was a total failure; your more restrained approach is the right way to go.

@timonegk timonegk force-pushed the catkin_test branch 2 times, most recently from dcd8730 to 29451b0 Compare April 19, 2021 12:15
@timonegk timonegk marked this pull request as ready for review April 20, 2021 08:18
@lilioid
Copy link

lilioid commented Apr 20, 2021

I cannot attest to the complete correctness or code styles but I have tested this PR locally on some packages with different options and it works very well for me.

@timonegk timonegk force-pushed the catkin_test branch 3 times, most recently from 9de9d40 to a97f74d Compare April 21, 2021 12:09
@Axel13fr
Copy link

Axel13fr commented Jul 5, 2021

Cheers @timonegk, I've used it on a couple of packages and that looked good to me.
Tried with -i, -p options on catkin ROS packages with gtest and rostests.

@asherikov
Copy link

I think --make-args is necessary for this verb still, for example in order to pass ctest parameters which are accepted by cmake test target via ARGS: make test ARGS="--output-log a.log"

@timonegk
Copy link
Member Author

Thanks for all of the positive feedback! I just added the --make-args option, maybe you could test if it works correctly @asherikov. I would like to open a new issue for the last point that is still unchecked (making the output directory configurable) to be done later and get this merged soon, as it is already a huge improvement over the existing state.

@asherikov
Copy link

Thanks,

  • catkin test --test-target test --make-args "ARGS=--output-log $(pwd)/a.log --verbose" -- <pkg> -- works
  • catkin test --test-target test --make-args "ARGS=--output-log $(pwd)/a.log --verbose" -j5 -- <pkg> -- does not, unrecognized option, not relevant in this case since the number of jobs can be set with ctest parameter, but may be there are other useful make parameters.

@LucasHaug
Copy link

Tested here and works very well!! Congrats

@timonegk
Copy link
Member Author

timonegk commented Oct 3, 2021

@asherikov, I fixed your issue. I am going to merge this now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants