make unit_test and boost_unit_test_framework optional

Bug #619045 reported by Nasenbaer
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Fix Released
High
Unassigned

Bug Description

The user Lukas mentioned on the German forums, that he has no clue how to get boost_unit_test installed on ubuntu. Because of that (as he did not know about the -DBUILD_UNIT_TESTS=False command) he was unable to build Widelands, although all *really* needed libraries and tools were installed.

I think that unit_tests should be an extra build option and only that build option should require boost_unit_test - it's a dev. feature and not a enduser feature.

Revision history for this message
SirVer (sirver) wrote :

While I disagree that unittests are a development feature (It saved my bacon so often before, when users could just say: hey i have some problems on my system and this test is also failing); but since we have so few tests I agree that we should disable them at least for release builds. Or even for all builds, I do not care.

Jens, I assigned you as you are the one cmake specialist we have :). Do you have time to take a look at this?

Changed in widelands:
assignee: nobody → Jens Beyer (Qcumber-some) (qcumber-some)
Revision history for this message
Jens Beyer (qcumber-some) wrote :

This should be done quite easily. Will add it to the spice-up-cmake branch to avoid conflicts with merging.

Revision history for this message
Jens Beyer (qcumber-some) wrote :

Just a little more thoughts:

The tests are a developer and a user feature.
Best example is Gentoo - usually packages offer a config option which can be enabled using the 'test' USE flag. Gentoo users are encouraged to have that flag enabled to automatically validate the compilation results before installing.

Making that configurable even for Release builds is definitely the better option than linking the tests to the Debug build.

Revision history for this message
Jari Hautio (jarih) wrote :

It's easiest just to change default value of BUILD_UNIT_TESTS to false, and trunk wont build tests nor require unit test framework unless tests are requested. As this value is cached, changing default value affects only when running cmake to new build directory.

For other tests approach that is used in src/scripting/CMakeLists.txt is propably fine: test sub directory is only included if BUILD_UNIT_TESTS is true.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2010-09-03 21:13:43 +0000
+++ CMakeLists.txt 2010-09-16 19:18:51 +0000
@@ -106,7 +106,7 @@
 set (BUILD_SHARED_LIBS OFF)

 # Unit tests add dependency to unit_test_framework and are therefore optional.
-set (BUILD_UNIT_TESTS TRUE CACHE BOOL "Build unit tests (requires boost unit test framework)")
+set (BUILD_UNIT_TESTS FALSE CACHE BOOL "Build unit tests (requires boost unit test framework)")

 if (BUILD_UNIT_TESTS)
   # we only include Boost Headers to the main executable, no libraries

Revision history for this message
Jari Hautio (jarih) wrote :

Maybe I should add that widelands itself is not linking with boost unit_test_framework. Only test executables link with boost unit test framework.

There's line in src/CMakeLists.txt:
target_link_libraries(widelands ${Boost_unit_test_framework_LIBRARIES})

But that does nothing because variable is always empty. Unit test framework libraries are in Boost_unit_test_framework_LIBRARY variable.

Revision history for this message
Jari Hautio (jarih) wrote :

As I did not found any fix in repos yet, I committed a fix to trunk r5564 to get this done. Initially I left the default option to include unit tests feeling that it's easy to switch it off if problems arise. But as it caused proplems, I felt I should see this fix to end.

Revision history for this message
Jens Beyer (qcumber-some) wrote :

I've done a modification of the trunk version in the spice-up-cmake branch.
Now it is implemented in the following way:

- Unit testing is enabled by default for Debug builds and disabled for Release builds.
- For Debug builds WL_UNIT_TESTS can be set to OFF to disable testing. This removes the dependency to boost_unit_test_framework, but of course not the dependency to boost itself
- For Release builds, WL_UNIT_TESTS can be set to ON to enable automatic testing.
- You can also say "make test" if WL_UNIT_TESTS is enabled (by default or automatic)

Changed in widelands:
status: Confirmed → In Progress
Changed in widelands:
status: In Progress → Fix Committed
assignee: Jens Beyer (Qcumber-some) (qcumber-some) → nobody
Revision history for this message
SirVer (sirver) wrote :

Released in build16-rc1

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.