diff --git a/.ci/Ubuntu26.04/Dockerfile b/.ci/Ubuntu26.04/Dockerfile index 96dd10763..3166b7eb4 100644 --- a/.ci/Ubuntu26.04/Dockerfile +++ b/.ci/Ubuntu26.04/Dockerfile @@ -9,6 +9,7 @@ RUN apt-get update && \ cmake \ file \ g++ \ + gcovr \ git \ libgl-dev \ liblzma-dev \ diff --git a/.ci/compile.sh b/.ci/compile.sh index de4fac004..606b9d201 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -9,14 +9,16 @@ # --server compiles servatrice # --test runs tests # --sanitize builds with AddressSanitizer and UndefinedBehaviorSanitizer, gcc/clang only +# --coverage instruments the test run for line coverage and prints a gcovr summary, gcc only + # --debug or --release sets the build type ie CMAKE_BUILD_TYPE # --ccache [] uses ccache and shows stats, optionally provide size # --evict-ccache runs ccache eviction based on given age after build # --dir sets the name of the build dir, default is "build" # --cmake-generator sets CMAKE_GENERATOR as used by cmake # --target-macos-version sets the min os version - only used for macOS builds -# uses env: BUILDTYPE MAKE_INSTALL MAKE_PACKAGE PACKAGE_TYPE PACKAGE_SUFFIX MAKE_SERVER MAKE_NO_CLIENT MAKE_TEST USE_CCACHE CCACHE_SIZE CCACHE_EVICTION_AGE BUILD_DIR CMAKE_GENERATOR TARGET_MACOS_VERSION -# (correspond to args: --debug/--release --install --package --suffix --server --test --ccache --dir ) +# uses env: BUILDTYPE MAKE_INSTALL MAKE_PACKAGE PACKAGE_TYPE PACKAGE_SUFFIX MAKE_SERVER MAKE_NO_CLIENT MAKE_TEST USE_CCACHE CCACHE_SIZE CCACHE_EVICTION_AGE BUILD_DIR CMAKE_GENERATOR TARGET_MACOS_VERSION MAKE_SANITIZE MAKE_COVERAGE +# (correspond to args: --debug/--release --install --package --suffix --server --test --sanitize --coverage --ccache --dir ) # exitcode: 1 for failure, 3 for invalid arguments # Read arguments @@ -62,6 +64,10 @@ while [[ $# != 0 ]]; do MAKE_SANITIZE=1 shift ;; + '--coverage') + MAKE_COVERAGE=1 + shift + ;; '--debug') BUILDTYPE="Debug" shift @@ -136,6 +142,15 @@ if [[ ! $BUILD_DIR ]]; then BUILD_DIR="build" fi mkdir -p "$BUILD_DIR" + +if [[ $MAKE_COVERAGE ]]; then + # Resolved while the working directory is still the source root. $0 is relative to the + # directory the script was invoked from, so after the cd below it no longer resolves, and + # under `set -e` the failed subshell would abort the build rather than just yielding an empty + # variable. Only the coverage report needs this, so only coverage pays for it. + SRC_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +fi + cd "$BUILD_DIR" # Set minimum CMake Version @@ -160,6 +175,9 @@ else # into the shipped binaries (libasan/libubsan Depends in the DEB). flags+=("-DENABLE_SANITIZERS=OFF") fi +if [[ $MAKE_COVERAGE ]]; then + flags+=("-DENABLE_COVERAGE=ON") +fi if [[ $USE_CCACHE ]]; then flags+=("-DUSE_CCACHE=1") # PCH-aware caching is required or ccache refuses to cache any TU that @@ -341,6 +359,31 @@ if [[ $MAKE_TEST ]]; then echo "::endgroup::" fi +if [[ $MAKE_COVERAGE ]]; then + # gcovr is not a build dependency and is not available everywhere, so a missing one is reported + # and skipped rather than failing an otherwise green build. + echo "::group::Coverage report" + GCOVR="$(command -v gcovr || true)" + if [[ -z $GCOVR ]]; then + echo "gcovr not found; install it to produce a coverage report" + else + "$GCOVR" --version + # This script configures with `cmake ..`, so the build dir it was started from is the object + # directory and the source root is one level up. Both are passed explicitly: gcovr otherwise + # walks the current directory and trips over in-tree build directories left by other tooling, + # whose stale .gcno files cannot be resolved. --config is named too, so the filters apply no + # matter what directory gcovr happens to start from. + GCOVR_ARGS=("." --root "$SRC_ROOT" --object-directory "." --config "$SRC_ROOT/gcovr.cfg") + "$GCOVR" "${GCOVR_ARGS[@]}" --txt --decisions + "$GCOVR" "${GCOVR_ARGS[@]}" --html-details coverage-details.html --html-title "Cockatrice line coverage" + "$GCOVR" "${GCOVR_ARGS[@]}" --lcov -o coverage.lcov + "$GCOVR" "${GCOVR_ARGS[@]}" --cobertura-pretty -o coverage.xml + "$GCOVR" "${GCOVR_ARGS[@]}" --json-summary-pretty -o coverage-summary.json + echo "Coverage artifacts written to coverage-details.html, coverage.lcov, coverage.xml and coverage-summary.json" + fi + echo "::endgroup::" +fi + if [[ $MAKE_INSTALL ]]; then echo "::group::Install" cmake --build . --target install --config "$BUILDTYPE" diff --git a/.ci/docker.sh b/.ci/docker.sh index 46112daaa..412ad6a1d 100644 --- a/.ci/docker.sh +++ b/.ci/docker.sh @@ -13,11 +13,17 @@ # --set-cache sets the location to cache the image or for ccache # # requires: docker -# uses env: NAME CACHE BUILD GET SAVE INTERACTIVE +# uses env: NAME IMAGE_SUFFIX CACHE BUILD GET SAVE INTERACTIVE # (correspond to args: --set-cache --build --get --save --interactive) # sets env: RUN CCACHE_DIR IMAGE_NAME RUN_ARGS RUN_OPTS BUILD_SCRIPT # exitcode: 1 for failure, 2 for missing dockerfile, 3 for invalid arguments # +# IMAGE_SUFFIX is optional and only appended to the image name, never to NAME, so that a matrix +# leg can have its own image and image cache without needing its own directory under .ci. Two legs +# building and tagging the same image concurrently would race on `docker build --tag`. +# It is deliberately not called SUFFIX: compile.sh already uses SUFFIX for output file names, and +# a job-level SUFFIX would be silently overridden by those per-step env blocks. +# # exported RUN function will run the BUILD_SCRIPT inside of the docker container. # note that the docker container will not inherit any environment variables! # @@ -81,7 +87,7 @@ if ! [[ $NAME ]]; then return 3 fi -export IMAGE_NAME="${project_name,,}_${NAME,,}" # lower case +export IMAGE_NAME="${project_name,,}_${NAME,,}${IMAGE_SUFFIX:+_${IMAGE_SUFFIX,,}}" # lower case docker_dir=".ci/$NAME" if ! [[ -r $docker_dir/Dockerfile ]]; then diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index 57c2993f6..098aabc4f 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -149,17 +149,31 @@ jobs: # uninstrumented suite, so nothing is lost by instrumenting this one. sanitize: yes - name: ${{ matrix.distro }} ${{ matrix.version }} + - distro: Ubuntu + version: 26.04 + variant: cover # separate image and ccache, same distro + + # The one leg that runs instrumented for line coverage and publishes the report as + # a workflow artifact. It is a separate leg rather than a second flag on the + # sanitizer leg because the two instruments answer different questions: the + # sanitizer leg reports what the run did wrong, the coverage leg reports what the + # run never touched at all. Combining them would also slow both down. Packaging is + # skipped because the leg above already publishes the DEB for this distro. + coverage: yes + package: skip + + name: ${{ matrix.distro }} ${{ matrix.version }}${{ matrix.variant && format(' {0}', matrix.variant) || '' }} needs: configure runs-on: ubuntu-latest # https://github.com/actions/runner-images continue-on-error: ${{ matrix.allow-failure == 'yes' }} timeout-minutes: 70 env: - CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }} # directory for caching docker image and ccache + CACHE: ${{ github.workspace }}/.cache/${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} # directory for caching docker image and ccache CCACHE_EVICTION_AGE: 7d CCACHE_SIZE: 600M # space of all repo is 10Gi: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#usage-limits-and-eviction-policy CMAKE_GENERATOR: 'Ninja' NAME: ${{ matrix.distro }}${{ matrix.version }} + IMAGE_SUFFIX: ${{ matrix.variant }} steps: - name: "Checkout" @@ -169,7 +183,7 @@ jobs: id: restore_ccache uses: actions/cache/restore@v6 with: - key: ccache-${{ matrix.distro }}${{ matrix.version }} + key: ccache-${{ matrix.distro }}${{ matrix.version }}${{ matrix.variant }} path: ${{ env.CACHE }} - name: "Build ${{ matrix.distro }} ${{ matrix.version }} Docker image" @@ -181,6 +195,7 @@ jobs: shell: bash env: sanitize: '${{ matrix.sanitize }}' + coverage: '${{ matrix.coverage }}' run: | source .ci/docker.sh args=() @@ -190,9 +205,32 @@ jobs: extra=() [[ $sanitize == yes ]] && extra+=(--sanitize) + [[ $coverage == yes ]] && extra+=(--coverage) RUN --server --debug --test "${extra[@]}" "${args[@]}" + # RUN bind-mounts the workspace into the container, so the report the build wrote into + # build/ is already on the host and no second container is needed to collect it. + - name: "Collect coverage report" + if: matrix.coverage == 'yes' + shell: bash + run: | + mkdir -p coverage + # --html-details writes one page per source file plus a stylesheet and script, and the + # index links to all of them by relative name. Copying the index alone would publish a + # report where every link 404s, so the whole family has to travel together. + cp build/coverage-details* coverage/ + cp build/coverage.lcov build/coverage.xml build/coverage-summary.json coverage/ + ls -la coverage/ + + - name: "Upload coverage artifact" + if: matrix.coverage == 'yes' + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage/ + if-no-files-found: warn + - name: "Build release package" id: build if: matrix.package != 'skip' diff --git a/CMakeLists.txt b/CMakeLists.txt index ef8866e3a..b272d4ad8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,8 @@ option(WITH_SERVER "Build Cockatrice server (Servatrice)" OFF) option(TEST "Build tests" OFF) # Build with AddressSanitizer and UndefinedBehaviorSanitizer option(ENABLE_SANITIZERS "Build with AddressSanitizer and UndefinedBehaviorSanitizer" OFF) +# Build instrumented for GCC line coverage, reported with gcovr +option(ENABLE_COVERAGE "Build instrumented for GCC line coverage" OFF) # Use vcpkg regardless of OS option(USE_VCPKG "Use vcpkg regardless of OS" OFF) @@ -213,9 +215,11 @@ else() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pipe") endif() -# Always included, because it defines cockatrice_disable_sanitizers_for() for the vendored -# trees. It only touches the global flags when ENABLE_SANITIZERS is on. +# Always included, because they define cockatrice_disable_sanitizers_for() and +# cockatrice_disable_coverage_for() for the vendored trees. They only touch the global flags +# when ENABLE_SANITIZERS or ENABLE_COVERAGE is on. include(Sanitizers) +include(Coverage) # GNU systems need to define the Mersenne exponent for the RNG to compile w/o warning if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") diff --git a/cmake/Coverage.cmake b/cmake/Coverage.cmake new file mode 100644 index 000000000..0345a2d36 --- /dev/null +++ b/cmake/Coverage.cmake @@ -0,0 +1,75 @@ +# Line-coverage instrumentation opt-in, plus per-source opt-out +# Input: ENABLE_COVERAGE +# Defines: cockatrice_disable_coverage_for +# +# Included once from the root CMakeLists.txt, immediately after the per-compiler flag +# block, so add_compile_options() reaches every library and executable defined below. +# +# Note the name deliberately shadows CMake's own built-in Modules/Coverage.cmake. It resolves +# to this file only because the root CMakeLists.txt puts cmake/ at the front of +# CMAKE_MODULE_PATH; if that ever moves, include(Coverage) would silently pick up the built-in +# module, which only defines a `COVERAGE` custom target and touches no global flags, so +# instrumentation would quietly stop happening rather than erroring. + +include(CheckCXXSourceCompiles) + +# Opt a set of vendored sources out of instrumentation. +# +# cockatrice_disable_coverage_for( ...) +# +# Third-party code drags the total down with lines no test can ever reach, and a total that +# cannot move is not a useful signal. This is the same carve-out the sanitizer build uses. +# +# The sources must be named relative to the directory that compiles them. CMake resolves a +# source-file property against the directory scope the target was declared in, so calling +# this from the root with absolute paths silently does nothing. That is why the call sites +# live next to the add_library/add_executable that pull the vendored code in. +# +# SKIP_PRECOMPILE_HEADERS mirrors the sanitizer helper. The precompiled header carries the +# instrumentation flags, and mixing an instrumented PCH with an uninstrumented translation +# unit makes GCC reject it. +function(cockatrice_disable_coverage_for) + if(NOT ENABLE_COVERAGE) + return() + endif() + # There is no -fno-coverage: --coverage is shorthand for -fprofile-arcs -ftest-coverage, so both + # halves have to be negated. CC rejects "-fno-coverage" outright, which is how the vendored C + # files below hit it first. + set_source_files_properties( + ${ARGN} PROPERTIES COMPILE_OPTIONS "-fno-profile-arcs;-fno-test-coverage" SKIP_PRECOMPILE_HEADERS ON + ) +endfunction() + +if(ENABLE_COVERAGE) + # Line coverage needs the compiler's own toolchain, so only GCC is supported. Clang's + # equivalent is llvm-cov, which reads a different intermediate format and would need a + # separate report path; rejecting it is better than emitting .gcno files that gcovr cannot + # read. + if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + message(FATAL_ERROR "ENABLE_COVERAGE requires GCC, but the compiler is ${CMAKE_CXX_COMPILER_ID}. " + "Clang coverage needs llvm-cov and is not wired up." + ) + endif() + + # Probe the way the flag will actually be used: compile AND link. A .gcno file comes from the + # compile step, but the runtime that writes .gcda files comes from the link step, so probing + # only the compile step would pass on a toolchain that cannot actually produce coverage. + set(_cockatrice_saved_required_flags "${CMAKE_REQUIRED_FLAGS}") + set(_cockatrice_saved_required_link_options "${CMAKE_REQUIRED_LINK_OPTIONS}") + set(CMAKE_REQUIRED_FLAGS "-O0 -g") + set(CMAKE_REQUIRED_LINK_OPTIONS -fprofile-arcs -ftest-coverage) + check_cxx_source_compiles("int main() { return 0; }" COCKATRICE_HAVE_COVERAGE_FLAGS) + set(CMAKE_REQUIRED_FLAGS "${_cockatrice_saved_required_flags}") + set(CMAKE_REQUIRED_LINK_OPTIONS "${_cockatrice_saved_required_link_options}") + + if(NOT COCKATRICE_HAVE_COVERAGE_FLAGS) + message(FATAL_ERROR "ENABLE_COVERAGE is on, but this toolchain rejected the coverage flags.") + endif() + + # -O0 keeps the line mapping honest: at higher optimisation levels the compiler merges and + # relocates lines, and the report describes code that no longer exists in that form. + add_compile_options(--coverage -O0 -g) + add_link_options(--coverage) + + message(STATUS "Coverage instrumentation enabled (--coverage). Report with gcovr over the build dir.") +endif() diff --git a/gcovr.cfg b/gcovr.cfg new file mode 100644 index 000000000..ac893b6a4 --- /dev/null +++ b/gcovr.cfg @@ -0,0 +1,63 @@ +# gcovr configuration for Cockatrice line coverage. +# +# Used by .ci/compile.sh --coverage and by hand: +# +# cmake -S . -B build/cover -DCMAKE_BUILD_TYPE=Debug -DWITH_SERVER=1 -DTEST=1 -DENABLE_COVERAGE=ON +# cmake --build build/cover -j 24 +# ctest --test-dir build/cover -LE bench -j 24 +# gcovr build/cover --root . --object-directory build/cover --txt --decisions +# +# The point of most of this file is subtraction. A raw report is dominated by code no test can +# reach: vendored third-party trees, build directories left over from earlier experiments, and +# the client's main() and window classes, which have no headless test. Those all dilute the one +# number worth watching, so they are filtered out here rather than in every invocation. + +# root is deliberately not set here. This file is normally invoked from the build directory +# (compile.sh runs there, because it configures with `cmake ..`), so a `root` of `.` would +# resolve to the build tree and every filter below would exclude everything. compile.sh passes +# --root explicitly instead. + +# Our own code only. This keeps generated Qt MOC output, protobuf output and anything CMake +# built under the source tree out of the report. +filter = ^cockatrice/ +filter = ^libcockatrice_[^/]+/ +filter = ^servatrice/ +filter = ^oracle/ + +# The tests are the vehicle, not the subject. They are compiled with the same instrumentation, +# so leaving them in would add their near-perfect coverage to the numerator and quietly report a +# healthier project than we have: test code is only ever executed by the run that is measuring +# it. Helpers under tests/ are excluded on the same grounds. +exclude = ^tests/ + +# Vendored third-party code is not ours to cover and is never going to move the number. +exclude = ^vcpkg/ +exclude = ^doc/doxygen/theme/ +exclude = ^libcockatrice_rng/libcockatrice/rng/sfmt/ +exclude = ^libcockatrice_utility/libcockatrice/utility/peglib\.h$ +exclude = ^oracle/src/lzma/ +exclude = ^oracle/src/qt-json/ +exclude = ^oracle/src/zip/ +exclude = ^servatrice/src/smtp/ + +# Qt's own moc/pch/uic output is generated into the build tree, not the source tree, so the +# filters above already keep it out. These cover CLion's directories, which sit in-tree and are +# picked up by the object-directory walk. +exclude = .*cmake-build-.* +exclude = .*\.cache/ + +# Process entry points and the client's window chrome. Each of these needs a display, an +# interactive terminal, or a live database before it does anything a test could observe, so no +# headless run will ever move them. Left in, they contribute thousands of lines at 0% and drag +# the total down by a fixed amount that hides movement in the code we can actually test. The +# window header is excluded with its .cpp because gcovr attributes its inline template code +# there, which is where most of its line count lives. +exclude = ^cockatrice/src/main\.cpp$ +exclude = ^cockatrice/src/interface/window_main\. +exclude = ^oracle/src/main\.cpp$ +exclude = ^servatrice/src/main\.cpp$ + +exclude-throw-branches = yes +exclude-function-lines = yes + +print-summary = yes diff --git a/libcockatrice_rng/CMakeLists.txt b/libcockatrice_rng/CMakeLists.txt index 16beffcfe..6128e3f6e 100644 --- a/libcockatrice_rng/CMakeLists.txt +++ b/libcockatrice_rng/CMakeLists.txt @@ -13,6 +13,7 @@ add_library( # SFMT is vendored and its own headers trip UBSan's alignment and vptr checks. cockatrice_disable_sanitizers_for(libcockatrice/rng/sfmt/SFMT.c) +cockatrice_disable_coverage_for(libcockatrice/rng/sfmt/SFMT.c) target_include_directories(libcockatrice_rng PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/oracle/CMakeLists.txt b/oracle/CMakeLists.txt index 9b6863553..0df345812 100644 --- a/oracle/CMakeLists.txt +++ b/oracle/CMakeLists.txt @@ -81,6 +81,7 @@ if(ZLIB_FOUND) list(APPEND oracle_SOURCES src/zip/unzip.cpp src/zip/zipglobal.cpp) # minizip and zlib are vendored. cockatrice_disable_sanitizers_for(src/zip/unzip.cpp src/zip/zipglobal.cpp) + cockatrice_disable_coverage_for(src/zip/unzip.cpp src/zip/zipglobal.cpp) else() message(STATUS "Oracle: zlib not found; ZIP support disabled") endif() @@ -93,6 +94,7 @@ if(LIBLZMA_FOUND) list(APPEND oracle_SOURCES src/lzma/decompress.cpp) # The LZMA SDK is vendored. cockatrice_disable_sanitizers_for(src/lzma/decompress.cpp) + cockatrice_disable_coverage_for(src/lzma/decompress.cpp) else() message(STATUS "Oracle: LibLZMA not found; xz support disabled") endif() diff --git a/servatrice/CMakeLists.txt b/servatrice/CMakeLists.txt index b176d8009..b45608d86 100644 --- a/servatrice/CMakeLists.txt +++ b/servatrice/CMakeLists.txt @@ -31,6 +31,9 @@ set(servatrice_RESOURCES servatrice.qrc) cockatrice_disable_sanitizers_for( src/smtp/qxthmac.cpp src/smtp/qxtmailattachment.cpp src/smtp/qxtmailmessage.cpp src/smtp/qxtsmtp.cpp ) +cockatrice_disable_coverage_for( + src/smtp/qxthmac.cpp src/smtp/qxtmailattachment.cpp src/smtp/qxtmailmessage.cpp src/smtp/qxtsmtp.cpp +) if(WIN32) set(servatrice_SOURCES ${servatrice_SOURCES} servatrice.rc) diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index c896c7960..6958673f7 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -26,6 +26,7 @@ if(ZLIB_FOUND) add_definitions("-DHAS_ZLIB") set(_ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/zip/unzip.cpp ../../oracle/src/zip/zipglobal.cpp) cockatrice_disable_sanitizers_for(${_ORACLE_BENCH_EXTRA_SOURCES}) + cockatrice_disable_coverage_for(${_ORACLE_BENCH_EXTRA_SOURCES}) set(_ORACLE_BENCH_EXTRA_LIBRARIES ${ZLIB_LIBRARIES}) include_directories(${ZLIB_INCLUDE_DIRS}) else() @@ -37,6 +38,7 @@ if(LIBLZMA_FOUND) add_definitions("-DHAS_LZMA") list(APPEND _ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/lzma/decompress.cpp) cockatrice_disable_sanitizers_for(../../oracle/src/lzma/decompress.cpp) + cockatrice_disable_coverage_for(../../oracle/src/lzma/decompress.cpp) list(APPEND _ORACLE_BENCH_EXTRA_LIBRARIES ${LIBLZMA_LIBRARIES}) include_directories(${LIBLZMA_INCLUDE_DIRS}) else()