Run tests against the installed version of the package (not the build/devel-time version)

[peci1]

In recent release of robot_body_filter, I forgot to add install directives to some newly added libraries. These libraries are required for the package to work (the main library links to them).

The package uses industrial CI to run tests and I thought this kind of issues would get caught in the CI. That means I expected that CI would install the package, then build the tests, and then run the tests against the installed version of the package. But e.g. build https://build.ros.org/job/Mbin_uB64__robot_body_filter__ubuntu_bionic_amd64__binary/101/ succeeded even with the missing directives, which could be explained by running the tests in the build/devel space instead.

I'm not sure if it's a bug in industrial CI or an expected behavior, but I'm reporting it anyways.

[gavanderhoorn]

I'm confused as to why you link to a build.ros.org job. industrial_ci is not used there.

[peci1]

You're right, that was a bad link. The Industrial CI job that succeeded on the flawed version is here: https://github.com/peci1/robot_body_filter/runs/3198426837?check_suite_focus=true .

[mathias-luedtke]

The package uses industrial CI to run tests and I thought this kind of issues would get caught in the CI.

In can be detected, but it depends on your configuration.

which could be explained by running the tests in the build/devel space instead.

As far as I know the tests are always run in the build/devel (?) space (for ROS1 < noetic).
This is even true for the ROS pre-release tests.
I am not 100% sure for ROS2, but I guess that there is a similar mechanism to find test nodes, which should not get installed.

Missing install tags can be detected, if you configure a DOWNSTREAM_WORKSPACE with a test package.

Another option is to try different builders, especially the isolated ones (including colcon).
The new TEST=debians (#719) might catch it as well, but I don't where the tests will be run (and if ^^).

And of course you should use CATKIN_LINT: true (or even CATKIN_LINT: pedantic) :)

[peci1]

Thanks for the analysis.

Yes, I'm now thinking about adding catkin lint which would probably catch this issue.

But having the failure directly as a result of the CI test would be much nicer. Would sourcing the install space just before running the tests help, or does the run_tests target do some kind of sourcing itself?

[mathias-luedtke]

does the run_tests target do some kind of sourcing itself?

I have not been able to find a definitive answer yet..
But I assume the tools always have an option to find files in the build space, just for practical reasons.
I can just say that industrial_ci does not source the anything before running the tests (except for the the upstream ROS install space).

Would sourcing the install space just before running the tests help

Just try it out:

BEFORE_RUN_TARGET_TEST_EMBED: extend="$(ici_extend_space "$target_ws")"
(please figure out how to escape it properly)

But having the failure directly as a result of the CI test would be much nicer.

Then you might want to create a dedicated test package.

[peci1]

Okay, I'll try to dig deeper when I have time - which might be a few months unfortunately. But I'm marking this as a thing to do.

Creating a dedicated test package sounds really cumbersome to me and not really the best way... Imagine every moveit package would need a dedicated test package to just test that the main package is installed correctly...

[mathias-luedtke]

Creating a dedicated test package sounds really cumbersome to me

Writing proper tests is always cumbersome! ;)
That's why catkin_lint is such a valuable helper!
But a test package even has the benefit of being a template for users.

Imagine every moveit package would need a dedicated test package to just test that the main package is installed correctly...

Not every package. If you have more than one package, then the install tags will get tested implicitly anyway.
ros_control is a good example for dedicated test packages.

and not really the best way...

I am open for suggestions :D

[peci1]

Just a short heads-up - I've noticed that noetic actually runs tests with installspace sourced. melodic, however, does not. I think the reason is that the noetic system has catkin/catkin_tools#676 while the melodic one does not. I've tried looking up the exact place in ici and catkin tools where the devel/install-space decision would be made, but unsucessfully... The only difference I've found is that catkin run_tests expands to catkin test on noetic and to catkin build --make-args run_tests on melodic.