From e564ec39b1013925344066191d980bc5206496b7 Mon Sep 17 00:00:00 2001
From: Ming Rui Zhang <mingrui.zhang@savoirfairelinux.com>
Date: Mon, 21 Jun 2021 17:18:52 -0400
Subject: [PATCH] test: building structure refinement

1. Add building tests only option in the build script
2. Google test intergration enhancement
3. Properly use QSignalSpy wait function

Change-Id: I5ec1b33e191cebd8a14c19d92ee4eb4d40f054c3
---
 make-client.py                       | 31 +++++++++++++++++---------
 tests/CMakeLists.txt                 | 33 +++++++---------------------
 tests/googletest/CMakeLists.txt.in   | 15 -------------
 tests/unittests/account_unittest.cpp | 12 +++++-----
 tests/unittests/contact_unittest.cpp | 20 ++++++++---------
 5 files changed, 45 insertions(+), 66 deletions(-)
 delete mode 100644 tests/googletest/CMakeLists.txt.in

diff --git a/make-client.py b/make-client.py
index f9c5f526f..af2478432 100644
--- a/make-client.py
+++ b/make-client.py
@@ -37,6 +37,11 @@ test_data_dir = temp_path + '\\jami_test\\jami'
 test_config_dir = temp_path + '\\jami_test\\.config'
 test_cache_dir = temp_path + '\\jami_test\\.cache'
 
+class TestBuilding(Enum):
+    NoTests = 0
+    WithTests = 1
+    OnlyTests = 2
+
 class QtVerison(Enum):
     Major = 0
     Minor = 1
@@ -201,7 +206,7 @@ def deps(arch, toolset, qtver):
 
 
 def build(arch, toolset, sdk_version, config_str, project_path_under_current_path, qtver,
-          enable_test, force_option=True):
+          test_building_type, force_option=True):
     print("Building with Qt " + qtver)
 
     configuration_type = 'StaticLibrary'
@@ -227,7 +232,7 @@ def build(arch, toolset, sdk_version, config_str, project_path_under_current_pat
         '-DQt5Gui_DIR=' + qt_cmake_dir + 'Qt5Gui',
         '-DQt5Test_DIR=' + qt_cmake_dir + 'Qt5Test',
         '-DQt5QuickTest_DIR=' + qt_cmake_dir + 'Qt5QuickTest',
-        '-DENABLE_TESTS=' + str(enable_test),
+        '-DENABLE_TESTS=' + (str("ENABLE_TESTS") if test_building_type != TestBuilding.NoTests else ''),
         '-DCMAKE_SYSTEM_VERSION=' + sdk_version
     ]
     if not os.path.exists(build_dir):
@@ -275,11 +280,11 @@ def build(arch, toolset, sdk_version, config_str, project_path_under_current_pat
     if (force_option):
         replace_necessary_vs_prop(project_path_under_current_path, toolset, sdk_version)
 
-    build_project(msbuild, msbuild_args, project_path_under_current_path, vs_env_vars)
+    if (test_building_type != TestBuilding.OnlyTests):
+        build_project(msbuild, msbuild_args, project_path_under_current_path, vs_env_vars)
 
     # build test projects
-
-    if (enable_test):
+    if (test_building_type != TestBuilding.NoTests):
         build_tests_projects(arch, config_str, msbuild, vs_env_vars,
                              toolset, sdk_version, force_option)
 
@@ -338,6 +343,9 @@ def parse_args():
     ap.add_argument(
         '-wt', '--withtest', action='store_true',
         help='Build Qt Client Test')
+    ap.add_argument(
+        '-ot', '--onlytest', action='store_true',
+        help='Build Only Qt Client Test')
     ap.add_argument(
         '-d', '--deps', action='store_true',
         help='Build Deps for Qt Client')
@@ -385,10 +393,13 @@ def main():
 
     parsed_args = parse_args()
 
-    enable_test = False
+    test_building_type = TestBuilding.NoTests
 
     if parsed_args.withtest:
-        enable_test = True
+        test_building_type = TestBuilding.WithTests
+
+    if parsed_args.onlytest:
+        test_building_type = TestBuilding.OnlyTests
 
     if parsed_args.subparser_name == 'runtests':
         run_tests(parsed_args.mutejamid, parsed_args.outputtofiles)
@@ -398,15 +409,15 @@ def main():
 
     if parsed_args.build:
         build(parsed_args.arch, parsed_args.toolset, parsed_args.sdk,
-              'Release', jami_qt_project, parsed_args.qtver, enable_test)
+              'Release', jami_qt_project, parsed_args.qtver, test_building_type)
 
     if parsed_args.beta:
         build(parsed_args.arch, parsed_args.toolset, parsed_args.sdk,
-              'Beta', jami_qt_project, parsed_args.qtver, enable_test)
+              'Beta', jami_qt_project, parsed_args.qtver, test_building_type)
 
     if parsed_args.releasecompile:
         build(parsed_args.arch, parsed_args.toolset, parsed_args.sdk,
-              'ReleaseCompile', jami_qt_project, parsed_args.qtver, enable_test)
+              'ReleaseCompile', jami_qt_project, parsed_args.qtver, test_building_type)
 
 
 if __name__ == '__main__':
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 620fda5da..71d98352c 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -1,32 +1,15 @@
 find_package(Qt5 CONFIG REQUIRED QuickTest Test)
 
 if(MSVC)
-    # Download and unpack googletest at configure time
-    configure_file(googletest/CMakeLists.txt.in googletest-download/CMakeLists.txt)
-    execute_process(COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" .
-                    RESULT_VARIABLE result
-                    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/googletest-download)
-    if(result)
-        message(FATAL_ERROR "CMake step for googletest failed: ${result}")
-    endif()
-        execute_process(COMMAND ${CMAKE_COMMAND} --build .
-                        RESULT_VARIABLE result
-                        WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/googletest-download)
-    if(result)
-        message(FATAL_ERROR "Build step for googletest failed: ${result}")
-    endif()
-
-    # Prevent overriding the parent project's compiler/linker
-    # settings on Windows
+    # Download and unpack googletest for windows
+    include(FetchContent)
+    FetchContent_Declare(
+        googletest
+        URL https://github.com/google/googletest/archive/master.zip
+    )
+    # for Windows: Prevent overriding the parent project's compiler/linker settings
     set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
-
-    # Add googletest directly to our build. This defines
-    # the gtest and gtest_main targets.
-    add_subdirectory(${CMAKE_CURRENT_BINARY_DIR}/googletest-src
-                     ${CMAKE_CURRENT_BINARY_DIR}/googletest-build
-                     EXCLUDE_FROM_ALL)
-
-    option(gtest_disable_pthreads "Disable uses of pthreads in gtest." ON)
+    FetchContent_MakeAvailable(googletest)
 else()
     find_package(GTest REQUIRED)
 endif()
diff --git a/tests/googletest/CMakeLists.txt.in b/tests/googletest/CMakeLists.txt.in
deleted file mode 100644
index 91cd0ef29..000000000
--- a/tests/googletest/CMakeLists.txt.in
+++ /dev/null
@@ -1,15 +0,0 @@
-cmake_minimum_required(VERSION 2.8.12)
-
-project(googletest-download NONE)
-
-include(ExternalProject)
-ExternalProject_Add(googletest
-                    GIT_REPOSITORY    https://github.com/google/googletest.git
-                    GIT_TAG           master
-                    SOURCE_DIR        "${CMAKE_CURRENT_BINARY_DIR}/googletest-src"
-                    BINARY_DIR        "${CMAKE_CURRENT_BINARY_DIR}/googletest-build"
-                    CONFIGURE_COMMAND ""
-                    BUILD_COMMAND     ""
-                    INSTALL_COMMAND   ""
-                    TEST_COMMAND      ""
-)
\ No newline at end of file
diff --git a/tests/unittests/account_unittest.cpp b/tests/unittests/account_unittest.cpp
index ea3a8816b..350363d4e 100644
--- a/tests/unittests/account_unittest.cpp
+++ b/tests/unittests/account_unittest.cpp
@@ -60,8 +60,8 @@ TEST_F(AccountFixture, CreateSIPAccountTest)
     // Create SIP Acc
     globalEnv.accountAdapter->createSIPAccount(QVariantMap());
 
-    if (accountAddedSpy.count() < 1)
-        QVERIFY(accountAddedSpy.wait());
+    QVERIFY(accountAddedSpy.wait());
+    QCOMPARE(accountAddedSpy.count(), 1);
 
     QList<QVariant> accountAddedArguments = accountAddedSpy.takeFirst();
     QVERIFY(accountAddedArguments.at(0).type() == QVariant::String);
@@ -76,8 +76,8 @@ TEST_F(AccountFixture, CreateSIPAccountTest)
     QSignalSpy accountStatusChangedSpy(&globalEnv.lrcInstance->accountModel(),
                                        &lrc::api::NewAccountModel::accountStatusChanged);
 
-    if (accountStatusChangedSpy.count() < 1)
-        QVERIFY(accountStatusChangedSpy.wait());
+    QVERIFY(accountStatusChangedSpy.wait());
+    QCOMPARE(accountStatusChangedSpy.count(), 1);
 
     // Remove the account
     globalEnv.lrcInstance->accountModel().removeAccount(
@@ -86,8 +86,8 @@ TEST_F(AccountFixture, CreateSIPAccountTest)
     QSignalSpy accountRemovedSpy(&globalEnv.lrcInstance->accountModel(),
                                  &lrc::api::NewAccountModel::accountRemoved);
 
-    if (accountRemovedSpy.count() < 1)
-        QVERIFY(accountRemovedSpy.wait());
+    QVERIFY(accountRemovedSpy.wait());
+    QCOMPARE(accountRemovedSpy.count(), 1);
 
     accountListSize = globalEnv.lrcInstance->accountModel().getAccountList().size();
     ASSERT_EQ(accountListSize, 0);
diff --git a/tests/unittests/contact_unittest.cpp b/tests/unittests/contact_unittest.cpp
index 0ab94fe40..cf293ef25 100644
--- a/tests/unittests/contact_unittest.cpp
+++ b/tests/unittests/contact_unittest.cpp
@@ -48,8 +48,8 @@ TEST_F(ContactFixture, AddSIPContactTest)
     // Create SIP Acc
     globalEnv.accountAdapter->createSIPAccount(QVariantMap());
 
-    if (accountAddedSpy.count() < 1)
-        QVERIFY(accountAddedSpy.wait());
+    QVERIFY(accountAddedSpy.wait());
+    QCOMPARE(accountAddedSpy.count(), 1);
 
     QList<QVariant> accountAddedArguments = accountAddedSpy.takeFirst();
     QVERIFY(accountAddedArguments.at(0).type() == QVariant::String);
@@ -61,8 +61,8 @@ TEST_F(ContactFixture, AddSIPContactTest)
     QSignalSpy accountStatusChangedSpy(&globalEnv.lrcInstance->accountModel(),
                                        &lrc::api::NewAccountModel::accountStatusChanged);
 
-    if (accountStatusChangedSpy.count() < 1)
-        QVERIFY(accountStatusChangedSpy.wait());
+    QVERIFY(accountStatusChangedSpy.wait());
+    QCOMPARE(accountStatusChangedSpy.count(), 1);
 
     // ModelUpdated signal spy
     QSignalSpy modelUpdatedSpy(globalEnv.lrcInstance->getCurrentContactModel(),
@@ -71,8 +71,8 @@ TEST_F(ContactFixture, AddSIPContactTest)
     // Add temp contact test
     globalEnv.lrcInstance->getCurrentConversationModel()->setFilter("test");
 
-    if (modelUpdatedSpy.count() < 1)
-        QVERIFY(modelUpdatedSpy.wait());
+    QVERIFY(modelUpdatedSpy.wait());
+    QCOMPARE(modelUpdatedSpy.count(), 1);
 
     QList<QVariant> modelUpdatedArguments = modelUpdatedSpy.takeFirst();
     QVERIFY(modelUpdatedArguments.at(0).type() == QVariant::String);
@@ -89,8 +89,8 @@ TEST_F(ContactFixture, AddSIPContactTest)
 
     globalEnv.lrcInstance->getCurrentConversationModel()->makePermanent(convId);
 
-    if (contactAddedSpy.count() < 1)
-        QVERIFY(contactAddedSpy.wait());
+    QVERIFY(contactAddedSpy.wait());
+    QCOMPARE(contactAddedSpy.count(), 1);
 
     // Remove the account
     globalEnv.lrcInstance->accountModel().removeAccount(
@@ -99,6 +99,6 @@ TEST_F(ContactFixture, AddSIPContactTest)
     QSignalSpy accountRemovedSpy(&globalEnv.lrcInstance->accountModel(),
                                  &lrc::api::NewAccountModel::accountRemoved);
 
-    if (accountRemovedSpy.count() < 1)
-        QVERIFY(accountRemovedSpy.wait());
+    QVERIFY(accountRemovedSpy.wait());
+    QCOMPARE(accountRemovedSpy.count(), 1);
 }
\ No newline at end of file
-- 
GitLab