Tests inside a static library #21

Closed
pthom opened this Issue Jun 4, 2016 · 24 comments

Projects

None yet

3 participants

@pthom
pthom commented Jun 4, 2016 edited

Hi,

(This is more a question than an issue)
What is the recommended way to add tests inside a static library project ?

If I compile a library (which includes TEST_CASE(s)) as a static library,
and then I try to run the tests from this library by creating a target that links
to it, the test suite is actually empty (i.e I guess the tests were stripped at link time)

Am I doing something wrong, or is there a way to circumvent this?

Below is an example of what I am trying to achieve (using cmake):

Project         <- main project
  CMakeLists.txt
  doctest/
    doctest.h
    doctest_main.cpp
  MyLibrary/
    lib1.cpp    <- these files includes actual library code and TEST_CASE(s)
    lib2.cpp    
    lib3.cpp    
    CMakeLists.txt <- will produce two targets : library and its test
  MyMainProgram/
    main.cpp
    CMakeLists.txt

MyLibrary/CMakeLists.txt :

    set(sources lib1.cpp lib2.cpp lib3.cpp)
    add_library(MyLibrary STATIC ${sources})
    target_include_directories(MyLibrary PUBLIC ${CMAKE_SOURCE_DIR}/doctest )

    # This does not work : it links but the test list is empty !
    add_executable(MyLibrary_DocTest ${CMAKE_SOURCE_DIR}/doctest/doctest_main.cpp)
    target_link_libraries(MyLibrary_DocTest MyLibrary)

    # This works, but it requires to re-compile all the source files and to re-define
    # exactly the same compile options as for the library (linked library, compile definition, etc); 
    # this can be tedious
    add_executable(MyLibrary_DocTest ${sources} ${CMAKE_SOURCE_DIR}/doctest/doctest_main.cpp)

MyMainProgram/CMakeLists.txt :

    add_executable(MyMainProgram main.cpp)
    target_link_libraries(MyMainProgram MyLibrary)

A working example can be found at https://github.com/pthom/DocTest_LibTest

@onqtam
Owner
onqtam commented Jun 4, 2016

well this is weird and unfortunate - will have to investigate this

@onqtam onqtam added the question label Jun 4, 2016
@martinmoene
Contributor

(Wild guess) If you're using Visual Studio, you may want to look at this note in the Google Test wiki (Mentioned here in the CATCH Google group).

@pthom
pthom commented Jun 4, 2016

@martinmoene : Thanks for the suggestion :-)
I use AppleClang, gcc and Visual Studio.
Actually, I observed the failure on a mac, using AppleClang.

@pthom
pthom commented Jun 4, 2016 edited

After testing, I can reproduce the problem with AppleClang, gcc 4.8 (ubuntu 64 bits), and MSVC 2013.

However, I think I found a good potential solution!

See pthom/DocTest_LibTest@e637352

If you add a dummy function in the static library and call it from the doctest main() function, then the tests will be run.
I just tested that it solves the issues with AppleClang, gcc 4.8 (ubuntu 64 bits), and MSVC 2013.

What do you think?
Hope this helps

@onqtam
Owner
onqtam commented Jun 4, 2016

seems like what the google.test docs say - will add it to my docs too - thanks!

@pthom
pthom commented Jun 5, 2016

Hi Viktor,
Hum, I thought my solution was sufficient, but it is not...

Each file containing tests needs to be "registered" separately, so that you have to create a dummy function in each file that contains tests, and call it from the main() function.
See for example : pthom/DocTest_LibTest@33df417

Based on my observations, the problem is not limited to Visual Studio (not anymore ?) : I can consistently reproduce it with AppleClang and gcc also...

A more generic way to test a static library could be to add a DocTestRegister() function to the library : this function would call all the dummy functions, and it would be called by doctest's main() function.
See pthom/DocTest_LibTest@5f42f75 for an example.

May be you will have a better idea (I hope so), because my solution is a bit tedious :-)

@pthom
pthom commented Jun 11, 2016

Hi Viktor,

Actually I fought to find a solution to this problem, and I made some helper cmake scripts for this. Would you mind giving me your opinion about them ? See https://github.com/pthom/doctest_registerlibrary

@onqtam onqtam removed the question label Jun 11, 2016
@onqtam
Owner
onqtam commented Jun 11, 2016 edited

I'll take a look - I do want to have a fix for this - this is the current thing I'm thinking about - thanks for the effort!

@onqtam
Owner
onqtam commented Jun 11, 2016 edited

omg! I did some research and there really is no way to fix this in a pretty way - the executable has to reference something from each object file from the static lib... (or if all object files from the static library reference each other like a linked list - then the executable needs to reference only 1 object file from the static library).

Anyway I'll try to solve this only in cmake and without the need to modify any of the source files - with compile definitions (by iterating through the source files of the static library and also based on their count - will give some identifier to the executable (like DOCTEST_NUM_DUMMIES=14 and I will try with macros to forward-declare and call 14 such dummies using the preprocessor) - the dummy functions will be like f12(), f13(), etc.). I should also think about the case when there are a few static libraries with tests - handling this counter will be tricky since it will have to be the total sum of source files in all static libraries... The identifiers used will be documented so if someone uses something other than cmake they should still be able to do this with the same identifiers in compile definitions.

I'll write here when it's done - in a few days.

If I fail - then your solution would be the best.

@pthom
pthom commented Jun 11, 2016

Hum, it seems I had an idea that was close to the python script you are mentioning, since I wrote some cmake functions that call a python script in order to generate the code that makes self registering work.

Look at the repo I sent you, it might help you; and if you like the idea just let me know, I'd like to be able to help. If you do not like it, no worries though :-)

@pthom
pthom commented Jun 11, 2016

Your approach is interesting ! Good luck with the preprocessor though, it is a difficult beast :-) your library proves that you are better at it than me

@pthom
pthom commented Sep 8, 2016

Hi,
Any news on your progress ?
If you are interested in my approach (and that is a big if ;-) , feel free to reference it or to copy it directly.
Cheers

@onqtam
Owner
onqtam commented Sep 8, 2016

:D I'm still delaying the finish of the 1.1 release. Actually my approach wouldn't work. I came up with a different one - if it doesn't work as well I will link in my FAQ to your project.

I hope that I release 1.1 in the next 1-2 weeks with this fixed (or linked to your solution)...

@pthom
pthom commented Sep 8, 2016

Thanks for the update. Keep me posted :D

@onqtam onqtam changed the title from Tests inside a library to Tests inside a static library Sep 9, 2016
@onqtam onqtam added a commit that referenced this issue Sep 9, 2016
@onqtam Using a CMake function called "doctest_force_link_static_lib_in_targe…
…t" located in "doctest_force_link_static_lib_in_target.cmake" users of CMake can force the linking of every object file from a static library to an executable/shared target. Not intrusive - using generated dummy headers in the build folder of the target, CMake target properties and compiler flags for including the headers to the appropriate sources.

fixes #21
4041b96
@onqtam
Owner
onqtam commented Sep 9, 2016 edited

Sooo I think I solved the issue! Here is the commit message:

Using a CMake function called "doctest_force_link_static_lib_in_target" like this located in examples/exe_with_static_libs/doctest_force_link_static_lib_in_target.cmake users of CMake can force the linking of every object file from a static library to an executable/shared target. Not intrusive - using generated dummy headers in the build folder of the target, CMake target properties and compiler flags for including the headers to the appropriate sources.

Also it is fool proof - using "doctest_force_link_static_lib_in_target" on the same target/library pair won't cause trouble.

even the scenario of linking in 2 shared objects with the same static lib with tests will work - no dummy function clashes when using the 2 shared objects together.

what do you think? It's just a single .cmake file and it doesn't modify the source files or require python or a separate build step.

still in the dev branch tough...

EDIT: linux builds seem to fail - will investigate.

EDIT 2: fixed the builds - the problem was that old cmake versions didn't support some target properties. the script now requires atleast cmake 3.0

@pthom
pthom commented Sep 10, 2016

Hello,
Thanks for the update. I am a bit busy today, but I will look into it quickly :-)

@pthom
pthom commented Sep 11, 2016

Wow, you did some serious cmake trickery :-)

I tested it and it worked with the following combinations :

  • OSX / make, OSX / XCode
  • ubuntu 12 / gcc 4.8
  • Visual Studio 2010 and 2013

I also works when you add tests to a file that was not using them, without re-running cmake.

what do you think? It's just a single .cmake file and it doesn't modify the source files or require >python or a separate build step.

I think it's just great !

May I suggest one additional thing : I am a lazy guy, and I like "ready to use" solutions.
Would it be possible to provide a ready to use cmake script that could be included by developpers of static lib ?
I wrote a draft here : pthom@738c51a

What do you think ?

@onqtam
Owner
onqtam commented Sep 11, 2016 edited

I have a few thoughts on this:

  • doctest_addincludepath should be used on the test target as well and not only on the lib (or is the PUBLIC part in target_include_directories supposed to be for this? not really in touch with modern cmake yet)
  • in doctest_main.cpp you propagate the result from doctest with a return only when shouldExit() returns true - that's for query flags (and the exit flag), but normal test execution always returns 0 - but should also return res. For this boilerplate main calling shouldExit() can be skipped and just return context.run();.
  • doctest_main.cpp can be generated by cmake (as I do with the dummy headers) in the build folder of the newly created executable and then doctest_lib_location will be less likely needed - I don't like such required globals...
  • not fond of the idea it going in the root directory
  • this use case seems too niche, but it won't hurt to provide these functions :D
@pthom
pthom commented Sep 13, 2016

not fond of the idea it going in the root directory
this use case seems too niche, but it won't hurt to provide these functions :D

I agree. It might be enough to provide these functions as a code comment inside examples/exe_with_static_libs/readme.md

People who might need would then copy / paste / adapt to their needs

@onqtam onqtam added a commit that referenced this issue Sep 13, 2016
@onqtam added a utility cmake function that creates an executable for a stati…
…c library with tests

related to #21
bd35fad
@onqtam
Owner
onqtam commented Sep 13, 2016

so I added the doctest_make_exe_for_static_lib function.

It generates the main.cpp for the executable in the build folder.
It assumes that it can include the doctest header like this: #include "doctest.h" so it should be already added in the include directories...

Does this suit your needs?

@pthom
pthom commented Sep 14, 2016 edited

so I added the doctest_make_exe_for_static_lib function.
Thanks a lot !

It assumes that it can include the doctest header like this: #include "doctest.h" so it should be already added in the include directories...

I saw this, and I see no better solution since when a function comes from an included cmake, there is no way for this function to know in which folder it is implemented.
So, that'll do it !

Thanks for the great job!

@pthom
pthom commented Sep 14, 2016 edited

Hum...

I'm afraid I found an issue with your solution.
If the library uses precompiled headers, each source file that will "use" the precompiled header ("/Yu" flag) will not obey to the forced include directive you added ("/FI")

It's very easy to test : just create a dummy project with precompiled headers, change one of the file to "force include" a file with voluntary syntax errors => it will compile just fine.

I tested this on Visual Studio 2010 and 2013.

Though, it does work with libraries that do not use precompiled headers.

@onqtam
Owner
onqtam commented Sep 14, 2016 edited

I found this issue on the web but it isn't very clear.

I did find one configuration that works though:

// main.cpp
int main() { void f(); f(); }
// a.cpp
void f() { printf("%d", from_force_included); }
// b.cpp
// c.cpp
// pch.h
#include <stdio.h>
// force_included.h
int from_force_included = 8;
# CMakeLists.txt
cmake_minimum_required(VERSION 3.0)

project(FI_TESTS)

include(../doctest/examples/exe_with_static_libs/doctest_force_link_static_lib_in_target.cmake)
include(../game/libs/third_party/ucm/cotire/CMake/cotire.cmake)

add_executable(FI_TESTS main.cpp a.cpp b.cpp c.cpp)

set_target_properties(FI_TESTS PROPERTIES COTIRE_ADD_UNITY_BUILD FALSE)
set_target_properties(FI_TESTS PROPERTIES COTIRE_CXX_PREFIX_HEADER_INIT "pch.h")

cotire(FI_TESTS) # used for precompiled headers

doctest_include_file_in_sources(force_included.h a.cpp)

The above example works only in this configuration. cotire is used for pch stuff.

If I change the order of the cotire() call and doctest_include_file_in_sources() then things break.

If I move the stuff from a.cpp to main.cpp and force include force_included.h in main.cpp instead of a.cpp things break again because main.cpp is 'compiling' the precompiled header (I guess since it's the first in the list of sources) - it has the /Yc option instead of `/Yu``.

Here are the compiler flags for a.cpp: /Yu"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.hxx" /FI"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.hxx" /FI"force_included.h" /Fp"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.pch"

and for main.cpp: /Yc"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.hxx" /FI"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.hxx" /Fp"D:\downloads\build\Debug\cotire\FI_TESTS_CXX_prefix.pch"

So in your case every source file from the static lib has a header force included into it without exception - including the source file that is 'compiling' the precompiled header - and that must be the result of the problems.

That's unfortunate... I'm all out of tricks.

Thanks for the huge donation! Hope you don't feel let down.

@pthom
pthom commented Sep 14, 2016

Hope you don't feel let down.
No, I don't, do not worry ;-)

@onqtam onqtam added a commit that referenced this issue Sep 15, 2016
@onqtam added comments - related to #21 5cadf1d
@onqtam onqtam added a commit that closed this issue Sep 21, 2016
@onqtam Using a CMake function called "doctest_force_link_static_lib_in_targe…
…t" located in "doctest_force_link_static_lib_in_target.cmake" users of CMake can force the linking of every object file from a static library to an executable/shared target. Not intrusive - using generated dummy headers in the build folder of the target, CMake target properties and compiler flags for including the headers to the appropriate sources.

fixes #21
a0e2846
@onqtam onqtam closed this in a0e2846 Sep 21, 2016
@onqtam onqtam added a commit that referenced this issue Sep 21, 2016
@onqtam added a utility cmake function that creates an executable for a stati…
…c library with tests

related to #21
837def0
@onqtam onqtam added a commit that referenced this issue Sep 21, 2016
@onqtam added comments - related to #21 154d5a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment