Look for system-installed googletest for use in Travis builds, drop embedded copies

[PeterBowman]

Currently, it's a common practice in repos that perform unit testing with Travis CI to host a gtest-x.x.x directory within test/, see kinematics-dynamics. Proposal: remove those files and install Google's testing framework via apt:

• https://packages.ubuntu.com/trusty/libgtest-dev (1.6.0!)
• https://packages.ubuntu.com/xenial/libgtest-dev (1.7.0)

---

• kinematics-dynamics (roboticslab-uc3m/kinematics-dynamics@93b37d4)
• tools (roboticslab-uc3m/tools@7eae586)
• xgnitive (roboticslab-uc3m/xgnitive@5d81fbc...06506e2)
• yarp-devices (roboticslab-uc3m/yarp-devices@dc8dcc6)
• mention in install guides (roboticslab-uc3m/installation-guides@1878368)

[PeterBowman]

Added TODO list in description. @jgvictores:

• OK to fiddle with xgnitive?
• yarp-devices is a bit different since its test scripts are ignored by Travis. Anyway, if we are to delete the gtest-* directory, we need to make sure that the libgtest-dev package is available. I did not find it in the archive Debian stretch database, hence it's marked as blocked for now until we move on to Xenial/Squeeze.

[jgvictores]

These months xgnitive is going to be a bit crazy, I'd wait till IROS 2018 deadline March 1 2018, ok?
I think tools is a great place to start; I'll be working on roboticslab-uc3m/kinematics-dynamics#134 at kinematics-dynamics but you can also work there as I'll just be touching TrajectoryLib on a separate branch.

[PeterBowman]

Funnily enough, libgtest-dev doesn't contain binaries we could use right away to build our unit tests. Per https://www.eriksmistad.no/getting-started-with-google-test-on-ubuntu/:

Note that this package only install source files. You have to compile the code yourself to create the necessary library files. These source files should be located at /usr/src/gtest. Browse to this folder and use cmake to compile the library:

The cost of not embedding GoogleTest framework in every single repository is... having to replicate the build instructions in .travis.yml to cover the compilation of GoogleTest itself.

[jgvictores]

Yes, the package description states Google's framework for writing C++ tests - header files.

I've been reading through a related bug report, where there was a proposal to take advantage of a gtest/gmock merge for a change... Only thing we'll see in the future for now is a name change from libgtest-dev to googletest but still no binaries in the file list.

Not sure what the best solution here is. I guess one practical solution would be to determine which method is faster, but that's only one of the possible criteria.

[jgvictores]

More bugs issued on Ubuntu end up pointing to bugs issued on Debian:

• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=726021
• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=845717

[jgvictores]

Okay, from ref 1 from above:

Because of the C++ "One Definition Rule", gtest must be compiled with exactly the same flags as your C++ code under test.

That... could actually make sense.

[David-Estevez]

You might already know this, but this is what Google recommends:

• Download the GoogleTest source code manually and place it at a known location. This is the least flexible approach and can make it more difficult to use with continuous integration systems, etc.
• Embed the GoogleTest source code as a direct copy in the main project's source tree. This is often the simplest approach, but is also the hardest to keep up to date. Some organizations may not permit this method.
• Add GoogleTest as a git submodule or equivalent. This may not always be possible or appropriate. Git submodules, for example, have their own set of advantages and drawbacks.
• Use CMake to download GoogleTest as part of the build's configure step. This is just a little more complex, but doesn't have the limitations of the other methods.

[PeterBowman]

Use CMake to download GoogleTest as part of the build's configure step. This is just a little more complex, but doesn't have the limitations of the other methods.

YCM could care about such complexity for us. Actually, I forgot that this was already an option (sorry!): #1 (comment).

I'm going to update the title accordingly.

[jgvictores]

Are we sure we do not prefer pulling the source via apt?

• Pro: looks easier
• Con: Older version, I guess

[PeterBowman]

It would force us to build and compile it by hand (in Travis, build steps should be carefully replicated) instead of letting CMake do its work (current behavior). Also, the package solution implies compiling with sudo, manually copying/symlinking binaries, etc. (example).

[jgvictores]

The example needs to compile with sudo and manually copy because it does an in-source build. I think we could do better.
Anyway, no personal preference. I guess since it's source and has to be compiled, it makes more sense to download the latest code anyway.

[PeterBowman]

it makes more sense to download the latest code anyway

I was thinking about making YCM pull & compile the exact same version we use right now (1.7.0).

[jgvictores]

I was thinking about making YCM pull & compile the exact same version we use right now (1.7.0).

👍

[PeterBowman]

It would force us to build and compile it by hand (in Travis, build steps should be carefully replicated) instead of letting CMake do its work (current behavior).

I'm still pondering on this, sorry. I'm definitely positive on using CMake somehow, although the YCM solution might be a bit too convoluted. Vanilla CMake already offers a FindGTest.cmake module, I'll probably test it soon.

[jgvictores]

I'm still pondering on this, sorry. I'm definitely positive on using CMake somehow, although the YCM solution might be a bit too convoluted. Vanilla CMake already offers a FindGTest.cmake module, I'll probably test it soon.

No worries! It's definitely a good idea to have thought it thoroughly, before propagating a solution throughout all repositories.

[PeterBowman]

FindGTest assumes that Google Test framework is already compiled, i.e. it looks for libgtest.so and libgtest_main.so (or .a) and reports an error if not found.

Because of the C++ "One Definition Rule", gtest must be compiled with exactly the same flags as your C++ code under test.

No good, then. Another option: install package and add_subdirectory(/usr/src/gtest).

[jgvictores]

Another option: install package and add_subdirectory(/usr/src/gtest)

Sounds okay. As long as we do an out-of-source build, we won't need the sudo for make.

[PeterBowman]

@jgvictores, @David-Estevez I followed the "package + vanilla CMake" solution: roboticslab-uc3m/kinematics-dynamics@f2ae4ed + Travis builds. What do you think? Some points to consider:

• Refactor and place the CMake code in a new find module, say FindGTestSources.cmake. This is different from standard FindGTest.cmake in the way that the latter looks for compiled binaries, whereas our module would determine the location of headers and sources (for use as input in add_subdirectory()).
• Said find module could borrow some ideas from the official FindGTest.cmake, such as support within CTest. Feels like reinventing the wheel, though.
• I tested that commit with googletest both as an apt package and cloned via git. The latter is especially relevant on Windows (users may set the GTEST_ROOT environment variable to point at the appropriate location). Should work nicely with googletest 1.6 and 1.7.
• It should be noted that googletest 1.8 changed a lot and new paths have to be considered in the find module to work properly. Affects Ubuntu packages from Zesty (17.04) onwards.
• Provide installation instructions for libgtest-dev in our guides.

As a side note: perhaps tests would be a better choice than test for the directory name of unit tests? We already have programs and libraries (but: cmake/template). I recall seeing a top-level src somewhere. In fact, directory layout might be better covered in a new issue in best-practices.

Edit: since a new dependency would be required to build unit tests (googletest package, no longer local), should they be disabled by default?

[PeterBowman]

Note to self: Establish gtest version (SO).

[David-Estevez]

Edit: since a new dependency would be required to build unit tests (googletest package, no longer local), should they be disabled by default?

What about disabling them if googletest cannot be found? (as devices do in yarp-devices if dependency is not found)

[PeterBowman]

I'm not sure about this. You'll usually want to build tests in Travis and abort them if not successful, other scenarios are non-critical. Silently hiding an unsatisfied dependency or even just warning about it is perhaps a bit too risky in this case.

Edit: see last comments at #18.

[PeterBowman]

I kinda like it. @jgvictores, @David-Estevez what do you think?

• FindGTestSources.cmake (supports googletest 1.6, 1.7, 1.8)
• usage in test/CMakeLists.txt (required 1.6)
• Travis build

As a side note: perhaps tests would be a better choice than test for the directory name of unit tests? We already have programs and libraries (but: cmake/template). I recall seeing a top-level src somewhere. In fact, directory layout might be better covered in a new issue in best-practices.

See #46.

Edit: since a new dependency would be required to build unit tests (googletest package, no longer local), should they be disabled by default?

You'll usually want to build tests in Travis and abort them if not successful, other scenarios are non-critical. Silently hiding an unsatisfied dependency or even just warning about it is perhaps a bit too risky in this case.

New issue?

What about disabling them if googletest cannot be found? (as devices do in yarp-devices if dependency is not found)

See #18.

[jgvictores]

IMHO, everything is fine. I'd even go for a solution like #18. Travis will fail with pretty obvious messages when attempting to run tests which have not been compiled if we ever forget to yamlYCM gtest.

[PeterBowman]

I'd even go for a solution like #18. Travis will fail with pretty obvious messages when attempting to run tests which have not been compiled if we ever forget to yamlYCM gtest.

Title updated, this is no longer a YCM issue according to the latest proposal.

I'm not sure about applying #18 here. We may forget to install libgtest-dev, but that should happen at most once. The question is: are unit tests valuable/relevant outside Travis CI? Should regular users care, and see annoying warnings about a missing package which would led them to dig into the installation guides?

PS sorry for "dizzying the partridge"!

[jgvictores]

Title updated, this is no longer a YCM issue according to the latest proposal.

Yes, I was about to do that, just couldn't think of an accurate description.

I'm not sure about applying #18 here. We may forget to install libgtest-dev, but that should happen at most once. The question is: are unit tests valuable/relevant outside Travis CI? Should regular users care, and see annoying warnings about a missing package which would led them to dig into the installation guides?

IMHO, the mechanism proposed at #18 only warns once, which isn't annoying. We are all developers and I think it's good for us to be informed if tests are not going to be compiled on our machine and why.
Workflow: get tired of warnings -> install gtest -> have tests compiled which at least checks if we break an API.

PS sorry for "dizzying the partridge"!

Big LOL!

[PeterBowman]

Done at kinematics-dynamics, added install guide:

• roboticslab-uc3m/kinematics-dynamics@93b37d4
• roboticslab-uc3m/installation-guides@1878368

[PeterBowman]

These months xgnitive is going to be a bit crazy, I'd wait till IROS 2018 deadline March 1 2018, ok?

@jgvictores OK! All other tasks are done, I leave xgnitive up to you. Closing issue.

[PeterBowman]

XGNITIVE done at roboticslab-uc3m/xgnitive@5d81fbc...06506e2.

[jgvictores]

Thanks a lot!!

[PeterBowman]

Note that googletest 1.8.0 forces installation of headers and generated binaries (google/googletest@98d988d). This behaviour may be suppressed on current googletest master (1.9.0?) with the INSTALL_GTEST CMake option (google/googletest@0e8e0e0), although it won't affect us since we don't process the root CMakeLists.txt where this option is defaulted to ON (FindGTestSources.cmake skips this directory and looks for googletest/CMakeLists.txt).

Affects artful (17.10) and bionic (18.04) Ubuntu distros (ref) as well as regular git clones (as recommended on Windows platforms). Things get a bit weird on the former: copy GTest headers into /usr/include/gtest as a result of installing libgtest-dev, process googletest's sources as part of another project (e.g. kinematics-dynamics), install it, and you'll get a copy of said headers at /usr/local/include/gtest.

[PeterBowman]

Alternatively, use FetchContent module from CMake 11. This allows to fetch external projects on configure time. Added in YCM 0.10 (not released yet).

[PeterBowman]

Note that googletest 1.8.0 forces installation of headers and generated binaries (google/googletest@98d988d). This behaviour may be suppressed on current googletest master (1.9.0?) with the INSTALL_GTEST CMake option (google/googletest@0e8e0e0), although it won't affect us since we don't process the root CMakeLists.txt where this option is defaulted to ON (FindGTestSources.cmake skips this directory and looks for googletest/CMakeLists.txt).

Things are getting weird on cosmic (18.10) and disco (19.04), the libgtest-dev package is no longer a mere placeholder for googletest (bionic, although it still depends on it) and now contains binaries along with gtest headers.

Alternatively, use FetchContent module from CMake 11. This allows to fetch external projects on configure time. Added in YCM 0.10 (not released yet).

This is currently broken, a fix is expected for YCM v0.10.2: robotology/ycm-cmake-modules#247.