[GNC-dev] Rework GoogleTest integration

[Christian Gruber]

Following my previous thread "[GNC-dev] Contribute to GnuCash 
development" I opened a new topic thread about reworking GoogleTest 
integration.


At first some investigation results on bug 797344 
<https://bugs.gnucash.org/show_bug.cgi?id=797344>.

In function gnc_gtest_configure() in file 
common/cmake_modules/GncAddTest.cmake the two CMake variables 
GTEST_INCLUDE_DIR and GMOCK_INCLUDE_DIR are set as follows:

find_path(GTEST_INCLUDE_DIR gtest/gtest.h HINTS ${GTEST_ROOT}/include 
${GMOCK_ROOT}/gtest/include /usr/include)
find_path(GMOCK_INCLUDE_DIR gmock/gmock.h HINTS ${GMOCK_ROOT}/include 
/usr/include)

This means, as long as GTEST_ROOT and GMOCK_ROOT are defined and refer 
to a valid GoogleTest repository, header files gtest.h and gmock.h are 
found there and the two variables GTEST_INCLUDE_DIR and 
GMOCK_INCLUDE_DIR will refer to the include directories within this 
GoogleTest repository.

In contrast to that the four CMake variables GTEST_MAIN_LIB, 
GTEST_SHARED_LIB, GMOCK_MAIN_LIB and GMOCK_SHARED_LIB are set in the 
same function gnc_gtest_configure() as follows:

find_library(GTEST_MAIN_LIB gtest_main)
find_library(GTEST_SHARED_LIB gtest)
find_library(GMOCK_MAIN_LIB gmock_main)
find_library(GMOCK_SHARED_LIB gmock)

This means, libraries are always searched in the default CMake search 
paths. Therefore any preinstalled GoogleTest libraries will be found 
this way.

This explains the behaviour described in my bug report.


Now let's come to my ideas regarding rework of GoogleTest integration. 
After further studying CMake files I have two general questions at first.

1. Why are library targets "gtest" and "gmock" defined in GnuCash build 
system instead of importing them from GoogleTest build results?

In file common/test-core/CMakeLists.txt these two targets are defined 
via add_library(). The same is done within GoogleTest build system in 
files googletest/CMakeLists.txt and googlemock/CMakeLists.txt. So part 
of the CMake build system from GoogleTest repository is copied into 
GnuCash build system.

My idea is to build and install GoogleTest completely independent from 
GnuCash using its own build system and then importing targets via 
find_package() into GnuCash build system. GoogleTest build system 
provides CMake configuration files after installation ready for 
importing targets into other projects.

CMake function find_package() is able to find package GTest via CMake 
configuration files, i.e. prebuilt from source code repository, as well 
as preinstalled libraries from distro using internal CMake module 
FindGTest <https://cmake.org/cmake/help/v3.5/module/FindGTest.html>. 
Beginning from CMake version 3.5 module FindGTest provides imported 
targets GTest::GTest and GTest::Main.

This would avoid mixing of libraries and header files from different 
locations.

Additionally the user doesn't have to define extra environment variables 
GTEST_ROOT and GMOCK_ROOT anymore. Instead the user can configure CMake 
search path using CMake variable CMAKE_PREFIX_PATH, if desired.

And the user can choose, which GoogleTest installation should be used by 
influencing the CMake search path. So the requirement, that one can 
decide whether to use preinstalled GoogleTest libraries from distro or 
GoogleTest installation prebuilt from sources would be fulfilled.


2. Why is library target "gtest" not used directly instead of variables 
GTEST_LIB, GTEST_INCLUDE_DIR?

Several test applications are defined via function gnc_add_test() by 
passing a list of sources, include directories and libraries to that 
function. This function gnc_add_test() is defined in file 
common/cmake_modules/GncAddTest.cmake and passes the list of libraries 
(TEST_LIBS_VAR_NAME) after resolving the variable names to CMake 
function target_link_libraries().

My idea is to add library target "gtest" directly to that list of 
libraries instead of variable GTEST_LIB, which is possible since CMake 
function target_link_libraries() can handle libraries as well as CMake 
targets. The advantage is, that you don't have to handle include 
directories separately on your own. When passing CMake targets to 
function target_link_libraries() CMake does all the stuff for you. 
Include directories defined for that target are added automatically, 
i.e. target_include_directories() doesn't have to be invoked to add 
GTEST_INCLUDE_DIR.

A further advantage is, that all test application targets depend on 
library target "gtest". And if library target "gtest" is not an imported 
target, it would be rebuilt automatically if necessary, when building 
test applications. Currently after any change to GoogleTest sources, 
e.g. checking out another version, you have to rebuilt GoogleTest 
libraries at first via "make gtest" for instance before you can rebuilt 
test applications via "make check" for instance. And if you forget that, 
you get a mix of GoogleTest header files and libraries from different 
versions.

Unfortunatelly in case of an imported target (see my first question), 
automatic rebuilt is not easily possible. In this case you have to 
define some extra custom command to invoke external GoogleTest build system.


These two questions are important to be answered before proceeding. Are 
there any considerations or constraints, which have to be taken into 
account, e.g. backwards compatibility?


Last there is another question.

3. Why has GTEST_SRC to be added to the list of sources passed to 
function gnc_add_test()?

Variable GTEST_SRC is set to ${GTEST_SRC_DIR}/src/gtest_main.cc in 
function gnc_gtest_configure() in file 
common/cmake_modules/GncAddTest.cmake. But as far as I can see this 
source file is already compiled and archived within library 
libgtest_main.a, which is also contained in list of libraries passed to 
function gnc_add_test() via variable GTEST_LIB.


Regards,
Christian