download_test_data: not documented and difficult to use

[jack-oquin]

There is no documentation for this command.

I am converting a rosbuild package to catkin. It used rosbuild_download_test_data(), which I changed to download_test_data().

The file gets downloaded to the build tree, but in rostest $(find PACKAGE)/tests/FILENAME.pcap can't find it there.

See this question for details, also the discussion in #416.

[jack-oquin]

I tried to hack around the problem by using an absolute file name in /tmp.

download_test_data(
    http://pr.willowgarage.com/data/velodyne/32e.pcap
    /tmp/${PROJECT}/tests/32e.pcap
    e41d02aac34f0967c03a5597e1d554a9)

But, download_test_data() insists on prefixing the name with ${PROJECT_BINARY_DIR}/, even when the it is fully qualified. It always goes in the build tree, although it cannot be resolved there.

[jack-oquin]

The following appalling hack seems to work:

set(SAVE_PROJECT_BINARY_DIR ${PROJECT_BINARY_DIR})
set(PROJECT_BINARY_DIR /tmp/velodyne_driver)
download_test_data(http://pr.willowgarage.com/data/velodyne/class.pcap
                   tests/class.pcap
                   65808d25772101358a3719b451b3d015)

download_test_data(http://pr.willowgarage.com/data/velodyne/32e.pcap
                   tests/32e.pcap
                   e41d02aac34f0967c03a5597e1d554a9)
set(PROJECT_BINARY_DIR ${SAVE_PROJECT_BINARY_DIR})

I even had to spell out the package name, because I could not get CMake to resolve /tmp/${PROJECT} correctly in the set() command.

[dirk-thomas]

The CMake variable for the project name is called PROJECT_NAME not PROJECT.

I don't have a package at hand which does it the way you describe it. Can you try the following (or point me to an existing repo with which I could give it a try)?

Since the test data is downloaded to a location relative to PROJECT_BINARY_DIR have you tried to register your test with a WORKING_DIRECTORY which points to PROJECT_BINARY_DIR? Than it might be possible to just reference the test data with a relative path (without any find logic).

[jack-oquin]

The package I am trying to port is the velodyne_driver.

To see how easily it all worked before, compare the rosbuild branch.

I have no idea what "register your test with a WORKING_DIRECTORY which points to PROJECT_BINARY_DIR" means. If you can tell me what to do, I will try it.

Since the file is used by rostest, I doubt a relative path name is going to work, however.

[dirk-thomas]

add_rostest has an optional argument WORKING_DIRECTORY like all the other CMake test functions. But this won't solve the problem since roslaunch will later run each node and test with the cwd pointing to ROS_HOME.

Taking a step back...

Any *.test file should be runnable via the rostest command line tool. Since this step does not perform the download of the test data it should be possible to pass an argument to rostest which gets passed on to the launch file which can than use $(arg) as it would do for normal launch files.

Than CMake can perform the download (to whatever location, preferably into build spacce) and pass that information to the rostest via add_rostest(test/custom.test ARGS foo:=bar)

That would cleanly separate the knowledge about the location of the test data to a single location - namely where it is downloaded and that location is passed to rostest.

I will look into making this possible with add_rostest.

[dirk-thomas]

I have added the feature to pass arbitrary arguments to add_rostest() using the ARGS keyword (see ros/ros_comm#232). Based on that you can modify the launch file to use an argument instead of a fixed path:

<param name="pcap" value="$(arg testdata)"/>

and in your CMake file you pass the location as a standard roslaunch argument:

add_rostest(tests/pcap_node_hertz.test ARGS "testdata:=${PROJECT_BINARY_DIR}/tests/class.pcap")

If you want to give this a try you need to place the latest rostest package (or a complete ros_comm checkout) into your workspace.

[jack-oquin]

Good idea, Dirk. The ARGS option looks like a generally useful extension.

Does it work for .launch files as well as .test files?

Where is add_rostest() defined? There's no documentation for it, either. I looked in catkin, but could not find it. Now I see from your update that it's buried in ros_comm.

I can give the new option a try, but won't be able to actually use it until you release a new ros_comm. I am still trying to release the Velodyne driver to Hydro. You probably would not believe how long that is taking me.

[dirk-thomas]

I think I don't get your question right? What do you mean with "does it work for .launch files as well"? The command line tools roslaunch as well as rostest already support passing arguments. The patch only exposes that to the CMake macro add_rostest().

You can expect a new release of ros_comm within a couple of days. It has already been quite some time since the last version and several fixes have piled up. I am currently only waiting for some changelog related helper scripts to be finished.

[jack-oquin]

I found another issue: several Velodyne packages share the same test files. They each end up declaring the same build target name, which CMake does not allow, of course.

For now, I am commenting out the downloads in velodyne_pointcloud and using the ones downloaded by velodyne_driver. That is very hackish, and probably will not work with your ARGS solution, since it would need the ${PROJECT_BINARY_DIR} for a different package.

It would be better for each package to download the files independently, but that would require a package name prefix for the _testname target, which looks do-able in the code.

[jack-oquin]

Since rostest can use .launch files as well as .test files, I was wondering if their arguments can be passed similarly.

I presume they can, but it doesn't hurt to ask.

[jack-oquin]

I read @wjwwood's explanation of the reasons why downloaded files go in build space and not devel space. I didn't find them very convincing.

Wouldn't this work much cleaner if the files were find-able in all the usual ways? Maybe the current command is somewhat hopeless, because too many other packages already work around its quirks. But, a new command or option to store the files in devel space really looks attractive to me. It would make porting from rosbuild much more straightforward.

[dirk-thomas]

The devel space has the disadvantage that these files would be exposed to every package. Imho the test data files should be private to the testing package and not made publicly available.

[wjwwood]

I don't really care if it ends up in build or devel space, I was simply trying to figure out from a higher level what would be most correct.

Either way the macro needs to create package name-spaced targets and the data needs to be easily discoverable, I am just not sure what the best solution for that is yet.

[jack-oquin]

"The devel space has the disadvantage that these files would be exposed to
every package. Imho the test data files should be private to the testing
package and not made publicly available."

But, don't forget my earlier note about several packages sharing the same
test data. While clean separation is desirable, we need to make sure it

works in a large catkin work-space, which is not currently the case.

joq

[dirk-thomas]

I have created a pull request which implements a cleaner CMake function for downloading test data. I enables all use cases mentioned in this thread (as far as I can see) by allowing custom download destinations / filenames. The signature also exposes the target name.

Please feel to give it a try and comment on the pull request #431.

[jack-oquin]

Looks good. I'll try it.

Thanks, Dirk.

[dirk-thomas]

Should be addressed by #431.