-
Notifications
You must be signed in to change notification settings - Fork 0
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
Look for system-installed googletest for use in Travis builds, drop embedded copies #41
Comments
Added TODO list in description. @jgvictores:
|
These months |
Funnily enough,
The cost of not embedding GoogleTest framework in every single repository is... having to replicate the build instructions in |
Yes, the package description states 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 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. |
More bugs issued on Ubuntu end up pointing to bugs issued on Debian: |
Okay, from ref 1 from above:
That... could actually make sense. |
You might already know this, but this is what Google recommends:
|
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. |
Are we sure we do not prefer pulling the source via
|
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 |
The example needs to compile with |
I was thinking about making YCM pull & compile the exact same version we use right now (1.7.0). |
👍 |
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. |
FindGTest assumes that Google Test framework is already compiled, i.e. it looks for
No good, then. Another option: install package and |
Sounds okay. As long as we do an out-of-source build, we won't need the |
@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:
As a side note: perhaps Edit: since a new dependency would be required to build unit tests (googletest package, no longer local), should they be disabled by default? |
Note to self: Establish gtest version (SO). |
What about disabling them if googletest cannot be found? (as devices do in yarp-devices if dependency is not found) |
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. |
I kinda like it. @jgvictores, @David-Estevez what do you think?
See #46.
New issue?
See #18. |
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 |
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"! |
Yes, I was about to do that, just couldn't think of an accurate description.
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.
Big LOL! |
Done at kinematics-dynamics, added install guide: |
@jgvictores OK! All other tasks are done, I leave xgnitive up to you. Closing issue. |
XGNITIVE done at roboticslab-uc3m/xgnitive@5d81fbc...06506e2. |
Thanks a lot!! |
Note that googletest 1.8.0 forces installation of headers and generated binaries (google/googletest@98d988d). This behaviour may be suppressed on current googletest 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 |
Alternatively, use FetchContent module from CMake 11. This allows to fetch external projects on configure time. Added in YCM 0.10 (not released yet). |
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.
This is currently broken, a fix is expected for YCM v0.10.2: robotology/ycm-cmake-modules#247. |
Currently, it's a common practice in repos that perform unit testing with Travis CI to host a
gtest-x.x.x
directory withintest/
, see kinematics-dynamics. Proposal: remove those files and install Google's testing framework viaapt
:The text was updated successfully, but these errors were encountered: