Adding unit tests framework to protocol codebase (#3878)

* progres

* works!

* added sample fractalclient test

* updated CMakeLists.txt file

* fixed linker problem

* fixed library not found error, now need to set entrypoint for test executable

* great news, everything works now!!!!

* minor changes

* rev1

* rev2, linting

* bug fix on windows build

* rev0.1

* rev1

* rev1.1

* fixed bugs introduced when shortening CmakeFiles

* added server unit testing

* prevent build of server unit tests on mac

* maybe fixed bug?

* Fix C++ linker issue with Debug/Release mode difference

* Fix C++ DXGI extern header problem

* fixed D3D11 import problem

* Move main.c headers back into .c file

* Add Fractal{Client,Server}Test to GHA yml files

* Fix linking issues with gtest, and organize Windows lib list

* clarify in GHA

* newline

Co-authored-by: Nicholas Pipitone <npip99@gmail.com>
Co-authored-by: Philippe Noël <21990816+philippemnoel@users.noreply.github.com>

.github/workflows/protocol-check-pr.yml

@@ -115,23 +115,23 @@ jobs:
 
       ############################# Unix Configuration Steps END #############################
 
-      - name: Build Protocol Client and Server on Windows
+      - name: Build and Test Protocol Client and Server on Windows
         if: runner.os == 'Windows'
         shell: cmd # Acts as Visual Studio Developer Command Prompt due to ilammy/msvc-dev-cmd@v1
         working-directory: protocol
         run: |
           mkdir build
           cd build
           cmake .. -DCMAKE_BUILD_TYPE=Debug -DCI=TRUE -G "Ninja" || EXIT /B 1
-          ninja FractalClient || EXIT /B 1
-          ninja FractalServer || EXIT /B 1
+          ninja FractalClient FractalClientTest || EXIT /B 1
+          ninja FractalServer FractalServerTest || EXIT /B 1
 
-      - name: Build Protocol Client on Linux Ubuntu
+      - name: Build and Test Protocol Client on Linux Ubuntu
         if: runner.os == 'Linux'
         working-directory: protocol
-        run: ./build_protocol_targets.sh --cmakebuildtype=Debug --cmakesetCI FractalClient
+        run: ./build_protocol_targets.sh --cmakebuildtype=Debug --cmakesetCI FractalClient FractalClientTest
 
-      - name: Build Protocol Client on macOS
+      - name: Build and Test Protocol Client on macOS
         if: runner.os == 'macOS'
         env:
           FRACTAL_OSX_SYSROOT: "/Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk" # Necessary for building on macOS
@@ -140,12 +140,12 @@ jobs:
           mkdir build
           cd build
           cmake .. -DCMAKE_BUILD_TYPE=Debug -DCI=TRUE
-          make FractalClient -j
+          make FractalClient FractalClientTest -j
 
       - name: Build Protocol Server on Linux Ubuntu
         if: runner.os == 'Linux'
         working-directory: protocol
-        run: ./build_protocol_targets.sh --cmakebuildtype=Debug FractalServer
+        run: ./build_protocol_targets.sh --cmakebuildtype=Debug FractalServer FractalServerTest
 
       # Verify clang-format on Linux
       - name: Verify Builds with clang-format
@@ -200,7 +200,7 @@ jobs:
       - name: Build Protocol Server in Docker Container
         if: runner.os == 'Linux'
         working-directory: protocol
-        run: ./build_protocol_targets.sh --cmakebuildtype=Release FractalServer
+        run: ./build_protocol_targets.sh --cmakebuildtype=Release FractalServer FractalServerTest
 
       # This ensures that all the build files from build_protocol_targets.sh have actually been generated
       - name: Ensure that all protocol build files are present

protocol/CMakeLists.txt

@@ -23,13 +23,7 @@ cmake_minimum_required(VERSION 3.15)
 cmake_policy(SET CMP0091 NEW)
 include(CheckLanguage)
 
-# Host is needed since CMAKE_SYSTEM_NAME is set by the project() command
-if (${CMAKE_HOST_SYSTEM_NAME} MATCHES "Darwin")
-    # Clang can't handle CXX for some reason
-    project(FractalProtocol LANGUAGES C)
-else ()
-    project(FractalProtocol LANGUAGES C CXX)
-endif ()
+project(FractalProtocol LANGUAGES C CXX)
 
 if (CMAKE_CONFIGURATION_TYPES)
     message(FATAL_ERROR "Detected multi-configuration generator! This CMake file does not support multi-configuration generators.")
@@ -78,6 +72,7 @@ set(META_AUTHOR_ORGANIZATION "Fractal")
 set(META_AUTHOR_DOMAIN       "https://github.com/fractal/fractal")
 
 set(CMAKE_C_STANDARD 99)
+set(CMAKE_CXX_STANDARD 14)
 
 # verbose cmake outputs, useful for debugging CMake issues such as linking
 set(CMAKE_VERBOSE_MAKEFILE OFF)
@@ -264,6 +259,16 @@ if ((NOT LIBMFX AND NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin"))
     message(FATAL_ERROR "Library LIBMFX was not found! ${CMAKE_SOURCE_DIR}/lib/${arch}/mfx/${CMAKE_SYSTEM_NAME}")
 endif()
 
+# GoogleTests should be included before we set CMAKE_C_COMPILER to 'clang'.
+include(FetchContent)
+FetchContent_Declare(
+    googletest
+    URL https://github.com/google/googletest/archive/refs/heads/master.zip
+)
+# From GoogleTests' official README: prevent overriding the parent project's compiler/linker settings
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
+
 #Set OSX compiler and SDK globally
 if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
     set(CMAKE_C_COMPILER clang)
@@ -284,11 +289,123 @@ include_directories(SYSTEM ${CMAKE_BINARY_DIR}/include/ffmpeg)
 
 # Here is where we tell cmake to look in subdirectories for further CMakeLists.txt
 add_subdirectory(fractal)
+
+# Set variables to be used by the CMakeLists in the subdirectories
+set(PLATFORM_INDEPENDENT_LIBS   fractalCore
+    fractalAudio
+    fractalVideo
+    fractalClipboard
+    fractalNetwork
+    fractalLogging
+    fractalUtils
+    ${STATIC_SDL2}
+    ${LIB_SENTRY}
+)
+set(MAC_SPECIFIC_CLIENT_LIBS       "-framework OpenGL"
+    "-framework VideoToolbox"
+    "-framework CoreMedia"
+    "-framework CoreAudio"
+    "-framework AudioToolbox"
+    "-framework IOKit"
+    "-framework ForceFeedback"
+    "-framework Carbon"
+    "-framework AppKit"
+    "-framework Metal"
+    "-framework QuartzCore"
+    objc
+    iconv
+    ${LIB_OPENSSL}
+)
+
+# MSVC C/C++ Standard Library linking, based on Debug/Release
+if (${CMAKE_BUILD_TYPE} MATCHES Debug)
+    set(WINDOWS_CORE_LIBS
+        ucrtd
+        libcmtd
+        libcpmtd
+        vcruntimed
+        msvcrtd
+        msvcmrtd
+        msvcprtd
+    )
+elseif(${CMAKE_BUILD_TYPE} MATCHES Release)
+    set(WINDOWS_CORE_LIBS
+        ucrt
+        libcmt
+        libcpmt
+        vcruntime
+        msvcrt
+        msvcmrt
+        msvcprt
+    )
+endif()
+
+# Windows Libs Shared by both server and client go here
+set(WINDOWS_CORE_LIBS
+    ${WINDOWS_CORE_LIBS}
+    ${LIB_OPENSSL}
+    oldnames # Needed by gtest for things like dup2/creat/etc
+    kernel32
+    gdi32
+    winmm
+    imm32
+    shell32
+    advapi32
+    ole32
+    oleaut32
+    opengl32
+    user32
+    uuid
+    version
+    ws2_32
+    shlwapi
+    crypt32
+    d3d11
+    dxgi
+    dxguid
+    shcore
+    DbgHelp
+    setupapi
+)
+
+# Client Specific windows libs go here
+set(WINDOWS_SPECIFIC_CLIENT_LIBS
+    ${WINDOWS_CORE_LIBS}
+)
+
 add_subdirectory(client)
 if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin") # Server does not make or compile on MacOS
     set(SERV "FractalServer")
+    set(PLATFORM_INDEPENDENT_SERVER_LIBS
+        fractalCore
+        fractalAudio
+        fractalVideo
+        fractalClipboard
+        fractalCursor
+        fractalInput
+        fractalNetwork
+        fractalLogging
+        fractalUtils
+        ${STATIC_SDL2}
+        ${LIB_SENTRY}
+        ${LIBMFX}
+    )
+    # Server Specific windows libs go here
+    set(WINDOWS_SPECIFIC_SERVER_LIBS
+        ${WINDOWS_CORE_LIBS}
+        wldap32
+        normaliz
+        winspool
+        comdlg32
+        odbc32
+        odbccp32
+        avrt
+        d3dcompiler
+    )
     add_subdirectory(server)
 endif()
+# Include the unit tests code
+add_subdirectory(test)
 
 if (WSL)
     message(STATUS "Native bash not found (only WSL), clang-tidy will not be an option")

protocol/client/CMakeLists.txt

@@ -101,17 +101,7 @@ endif()
 #[[
 ################## Linking platform independent libs ##################
 ]]
-target_link_libraries(FractalClient
-        fractalCore
-        fractalAudio
-        fractalVideo
-        fractalClipboard
-        fractalNetwork
-        fractalLogging
-        fractalUtils
-        ${STATIC_SDL2}
-        ${LIB_SENTRY}
-        )
+target_link_libraries(FractalClient ${PLATFORM_INDEPENDENT_LIBS})
 
 #[[
 ################## LINUX ##################
@@ -124,13 +114,12 @@ if (${CMAKE_SYSTEM_NAME} MATCHES "Linux")
             Xfixes)
     find_package(ALSA REQUIRED)
     include_directories(${X11_INCLUDE_DIR} ${ALSA_INCLUD_DIR})
-    target_link_libraries(FractalClient
-            ${X11_LIBRARIES}
+    target_link_libraries(FractalClient 
+           ${X11_LIBRARIES}
             ${X11_Xfixes_LIB}
             ${ALSA_LIBRARIES}
             crypto
-            m
-            )
+            m)
 endif()
 
 #[[
@@ -139,22 +128,7 @@ endif()
 if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
     target_sources(FractalClient PRIVATE native_window_utils_mac.m)
 
-    target_link_libraries(FractalClient
-        "-framework OpenGL"
-        "-framework VideoToolbox"
-        "-framework CoreMedia"
-        "-framework CoreAudio"
-        "-framework AudioToolbox"
-        "-framework IOKit"
-        "-framework ForceFeedback"
-        "-framework Carbon"
-        "-framework AppKit"
-        "-framework Metal"
-        "-framework QuartzCore"
-        objc
-        iconv
-        ${LIB_OPENSSL}
-        )
+    target_link_libraries(FractalClient ${MAC_SPECIFIC_CLIENT_LIBS})
 endif()
 
 #[[
@@ -177,31 +151,5 @@ if (${CMAKE_SYSTEM_NAME} MATCHES "Windows")
     endif()
 
     # Link to system libs
-    target_link_libraries(FractalClient
-            ${LIB_OPENSSL}
-            libvcruntime
-            libucrt
-            libcmt
-            kernel32
-            gdi32
-            winmm
-            imm32
-            shell32
-            advapi32
-            ole32
-            oleaut32
-            opengl32
-            user32
-            uuid
-            version
-            ws2_32
-            shlwapi
-            crypt32
-            d3d11
-            dxgi
-            dxguid
-            shcore
-            DbgHelp
-            setupapi
-            )
+    target_link_libraries(FractalClient ${WINDOWS_SPECIFIC_CLIENT_LIBS})
 endif()

protocol/client/client_utils.c

@@ -33,27 +33,35 @@ Includes
 #include <fractal/logging/error_monitor.h>
 #include <fractal/core/fractalgetopt.h>
 
-extern volatile char binary_aes_private_key[16];
-extern volatile char hex_aes_private_key[33];
-extern volatile char *server_ip;
-extern volatile int output_width;
-extern volatile int output_height;
-extern volatile char *program_name;
-extern volatile CodecType output_codec_type;
+// Taken from main.c
+volatile int server_width = -1;
+volatile int server_height = -1;
+volatile CodecType server_codec_type = CODEC_TYPE_UNKNOWN;
+
+volatile char binary_aes_private_key[16];
+volatile char hex_aes_private_key[33];
+volatile char *server_ip;
+volatile int output_width;
+volatile int output_height;
+volatile char *program_name = NULL;
+volatile CodecType output_codec_type = CODEC_TYPE_H264;
 extern volatile SDL_Window *window;
 
 extern volatile int max_bitrate;
 
+// From main.c
+volatile bool update_bitrate = false;
+
 // This variables should stay as arrays - we call sizeof() on them
-extern char user_email[FRACTAL_ARGS_MAXLEN + 1];
-extern char icon_png_filename[FRACTAL_ARGS_MAXLEN + 1];
+char user_email[FRACTAL_ARGS_MAXLEN + 1];
+char icon_png_filename[FRACTAL_ARGS_MAXLEN + 1];
 
 extern bool skip_taskbar;
 
-extern bool using_stun;
+bool using_stun = false;
 
-extern MouseMotionAccumulation mouse_state;
-extern volatile SDL_Window *window;
+MouseMotionAccumulation mouse_state = {0};
+volatile SDL_Window *window;
 
 extern unsigned short port_mappings[USHRT_MAX];
 

protocol/client/handle_server_message.c

@@ -31,12 +31,12 @@ Includes
 
 #include <stddef.h>
 
-extern bool exiting;
+bool exiting = false;
 extern int audio_frequency;
 extern volatile double latency;
 extern volatile int try_amount;
-extern volatile char *window_title;
-extern volatile bool should_update_window_title;
+volatile char *window_title;
+volatile bool should_update_window_title;
 
 /*
 ============================