Re: [GNC-dev] Rework GoogleTest integration

2019-08-15 Thread John Ralls



> On Aug 15, 2019, at 3:22 PM, Christian Gruber  
> wrote:
> 
> Am 15.08.19 um 00:23 schrieb Christian Gruber:
>> 
>>> I think our distro packagers would object to any option other than relying 
>>> on tools outside of the build system to provide the googletest sources and 
>>> maybe a prebuilt static lib. It's a one-off for the casual builder and 
>>> easily scripted for everyone else. Building gtest is also sufficiently 
>>> trivial that it's not all that interesting to call out to guests own 
>>> CMakeLists.txt instead of just building the two libraries in ours. OTOH 
>>> it's possible that might change at some point, so
>> 
>> Ok, so you want to keep build process as it is at the moment. In this case 
>> at least bug 797344 should be fixed. I'll provide a PR. And I recommend to 
>> use library target gtest directly instead of variables GTEST_LIB, 
>> GTEST_INCLUDE_DIR as stated above. The same I recommend for library target 
>> gmock. I'll add this to the same PR.
>> 
>> Finally I recommend to add a few more notes on the Wiki page 
>> https://wiki.gnucash.org/wiki/Google_Test. For me not familiar with the 
>> GnuCash build system it was not obvious at the beginning, that gtest is 
>> built inside GnuCash build system, because this is a little bit unusual. It 
>> could for instance be explained, how to build gtest and gmock from inside 
>> GnuCash build system via "make gtest" and "make gmock" and that this way one 
>> can test the correct CMake configuration for GnuCash regarding variables 
>> GTEST_ROOT and GMOCK_ROOT. The Wiki page only explains how to "test the 
>> installation" from inside the GoogleTest build system and tells that this is 
>> "not used in practice". But this doesn't really test the CMake configuration 
>> for GnuCash.
>> 
>>> 
>>> I don't think anyone's actually tried doing a mock with GMock yet. There 
>>> are hand-rolled ones in the QOF tests, but they use the old Glib test 
>>> facility. Most of the C++ work so far has been at the lowest levels so that 
>>> the C++ classes don't have any mockable dependencies. That will change when 
>>> we get to redoing the engine as we'll want to use mock for at least the 
>>> backend. We don't want to remove GMock from the dependencies.
>>> 
>>> I agree about libgtest_main.a. Would you like to make a PR to remove all of 
>>> the inclusion of gtest_main.cc and GTEST_SRC?
>> I opened PR https://github.com/Gnucash/gnucash/pull/552.
>>> 
>>> Regards,
>>> John Ralls
>>> 
>> Regards,
>> Christian
>> 
> I just realized now, after PR https://github.com/Gnucash/gnucash/pull/552 
> failed to build, that there is a significant difference between using 
> prebuilt GoogleTest libraries and building GoogleTest from source repository. 
> In the latter case libgtest_main.a doesn't exist at all, it isn't built 
> anywhere. And GTEST_LIB contains only libgtest.a in this case. Therefore I 
> created a new pull request https://github.com/Gnucash/gnucash/pull/555, which 
> removes GTEST_SRC and additionally adds source file gtest_main.cc to the 
> sources of libgtest.a. I also thought about creating a second target 
> gtest_main for building libgtest_main.a as in GoogleTest source repository, 
> but in the end I thought that's exaggerated, because both libraries are 
> always used in combination in GnuCash.

That's fine, and I've merged your PR.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel


Re: [GNC-dev] Rework GoogleTest integration

2019-08-15 Thread Christian Gruber

Am 15.08.19 um 00:23 schrieb Christian Gruber:


I think our distro packagers would object to any option other than 
relying on tools outside of the build system to provide the 
googletest sources and maybe a prebuilt static lib. It's a one-off 
for the casual builder and easily scripted for everyone else. 
Building gtest is also sufficiently trivial that it's not all that 
interesting to call out to guests own CMakeLists.txt instead of just 
building the two libraries in ours. OTOH it's possible that might 
change at some point, so


Ok, so you want to keep build process as it is at the moment. In this 
case at least bug 797344 should be fixed. I'll provide a PR. And I 
recommend to use library target gtest directly instead of variables 
GTEST_LIB, GTEST_INCLUDE_DIR as stated above. The same I recommend for 
library target gmock. I'll add this to the same PR.


Finally I recommend to add a few more notes on the Wiki page 
https://wiki.gnucash.org/wiki/Google_Test. For me not familiar with 
the GnuCash build system it was not obvious at the beginning, that 
gtest is built inside GnuCash build system, because this is a little 
bit unusual. It could for instance be explained, how to build gtest 
and gmock from inside GnuCash build system via "make gtest" and "make 
gmock" and that this way one can test the correct CMake configuration 
for GnuCash regarding variables GTEST_ROOT and GMOCK_ROOT. The Wiki 
page only explains how to "test the installation" from inside the 
GoogleTest build system and tells that this is "not used in practice". 
But this doesn't really test the CMake configuration for GnuCash.




I don't think anyone's actually tried doing a mock with GMock yet. 
There are hand-rolled ones in the QOF tests, but they use the old 
Glib test facility. Most of the C++ work so far has been at the 
lowest levels so that the C++ classes don't have any mockable 
dependencies. That will change when we get to redoing the engine as 
we'll want to use mock for at least the backend. We don't want to 
remove GMock from the dependencies.


I agree about libgtest_main.a. Would you like to make a PR to remove 
all of the inclusion of gtest_main.cc and GTEST_SRC?

I opened PR https://github.com/Gnucash/gnucash/pull/552.


Regards,
John Ralls


Regards,
Christian

I just realized now, after PR 
https://github.com/Gnucash/gnucash/pull/552 failed to build, that there 
is a significant difference between using prebuilt GoogleTest libraries 
and building GoogleTest from source repository. In the latter case 
libgtest_main.a doesn't exist at all, it isn't built anywhere. And 
GTEST_LIB contains only libgtest.a in this case. Therefore I created a 
new pull request https://github.com/Gnucash/gnucash/pull/555, which 
removes GTEST_SRC and additionally adds source file gtest_main.cc to the 
sources of libgtest.a. I also thought about creating a second target 
gtest_main for building libgtest_main.a as in GoogleTest source 
repository, but in the end I thought that's exaggerated, because both 
libraries are always used in combination in GnuCash.



Regards,
Christian

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel


Re: [GNC-dev] Rework GoogleTest integration

2019-08-14 Thread Christian Gruber

Am 14.08.19 um 05:09 schrieb John Ralls:



On Aug 13, 2019, at 2:54 PM, Christian Gruber  
wrote:

Am 13.08.19 um 02:45 schrieb John Ralls:

On Aug 12, 2019, at 3:12 PM, Christian Gruber  
wrote:

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 
.

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 
. 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

Re: [GNC-dev] Rework GoogleTest integration

2019-08-13 Thread John Ralls



> On Aug 13, 2019, at 2:54 PM, Christian Gruber  
> wrote:
> 
> Am 13.08.19 um 02:45 schrieb John Ralls:
>> 
>>> On Aug 12, 2019, at 3:12 PM, Christian Gruber  
>>> wrote:
>>> 
>>> 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 
>>> .
>>> 
>>> 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 
>>> . 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 lib

Re: [GNC-dev] Rework GoogleTest integration

2019-08-13 Thread Christian Gruber

Am 13.08.19 um 02:45 schrieb John Ralls:



On Aug 12, 2019, at 3:12 PM, Christian Gruber  
wrote:

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 
.

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 
. 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 

Re: [GNC-dev] Rework GoogleTest integration

2019-08-12 Thread John Ralls



> On Aug 12, 2019, at 3:12 PM, Christian Gruber  
> wrote:
> 
> 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 
> .
> 
> 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 
> . 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 "

[GNC-dev] Rework GoogleTest integration

2019-08-12 Thread 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 
.


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 . 
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