From 75beb7f302c0b65aa43ae7954660191187c474e7 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Wed, 4 Nov 2026 07:22:43 +0100 Subject: [PATCH] [CI] Run the Linux test suite against the release build (#7665) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [CI] Run the Linux test suite against the release build A packaging leg compiled the tree twice: once in Debug to run the suite, once in Release to package it. The suite is 655 tests that finish in four seconds, so the Debug build bought nothing the release build cannot provide - macOS and Windows have always tested their release build. The sanitizer and coverage legs keep their Debug build, because that is the configuration their instrumentation is for. Every other leg runs ctest against the release configuration, ahead of the package target, so a failing suite still stops the leg before anything is built into an installer. A leg that only tests, Arch, builds Release without the crash reporting SDK the packaging legs ship with. Debian 13 and Fedora 44 each drop a 15 minute compile from a job that gates a pull request. * [CI] Keep Fedora 44 and Arch on the Debug build Only the Debug configuration passes -Wall -Wextra -Werror, so moving those two legs to the release build would have stopped the newest distro toolchains from failing on a new warning. They keep the Debug compile and test master had; the other Linux legs still test the release build. Addresses review thread PRRT_kwDOACxbl86x2tOP (PR #7665). * [CI] Decide the release test run once per leg The step's if: and its shell both recomputed "does this leg test the release build", and the two could drift into a leg that builds a release it never tests. RUN_TESTS now holds that decision and both read it. It lives at job level because a step's own env is not visible to that step's if:. Addresses review thread PRRT_kwDOACxbl86x2tOV (PR #7665). --------- Co-authored-by: Lukas Brübach --- .github/workflows/desktop-build.yml | 45 +++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index df054ea26..16610d872 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -141,6 +141,7 @@ jobs: allow-failure: yes package: skip # We are packaged in Arch already + debug_test: yes # see the note on Fedora 44 below - distro: Servatrice_Debian version: 12 @@ -170,6 +171,11 @@ jobs: version: 44 package: RPM + # Arch and Fedora 44 are the two newest toolchains, and the -Wno-error= exceptions + # in CMakeLists.txt exist for exactly their warnings. They keep compiling Debug: + # only the Debug configuration passes -Werror, so these two are the legs where a + # new warning in either compiler still fails the build. + debug_test: yes - distro: Ubuntu version: 24.04 @@ -212,6 +218,15 @@ jobs: CMAKE_GENERATOR: 'Ninja' NAME: ${{ matrix.distro }}${{ matrix.version }} IMAGE_SUFFIX: ${{ matrix.variant }} + # Which build the suite runs in, decided once per leg here so the steps' if: conditions + # and their shells cannot drift apart. A step's own env is not visible to its if:, which + # is why these live at job level rather than on the release step. + # DEBUG_TEST the instrumented legs (symbols and instrumentation matter) and the two + # newest distro toolchains, which only fail on a warning in Debug. + # RUN_TESTS every other leg that tests runs the suite against the release build it may + # also package, so testing and packaging cost one compile instead of two. + DEBUG_TEST: ${{ matrix.sanitize == 'yes' || matrix.coverage == 'yes' || matrix.debug_test == 'yes' }} + RUN_TESTS: ${{ matrix.test != 'skip' && !(matrix.sanitize == 'yes' || matrix.coverage == 'yes' || matrix.debug_test == 'yes') }} steps: - name: "Checkout" @@ -228,8 +243,11 @@ jobs: shell: bash run: source .ci/docker.sh --build + # The instrumented legs and the two newest distro toolchains build and test Debug; every + # other leg runs its suite against the release build below, so testing and packaging cost + # one compile instead of two. - name: "Build debug and test" - if: matrix.test != 'skip' + if: env.DEBUG_TEST == 'true' shell: bash env: sanitize: '${{ matrix.sanitize }}' @@ -269,15 +287,24 @@ jobs: path: coverage/ if-no-files-found: warn - - name: "Build release package" + # One build serves both purposes: the suite runs against the same configuration that + # gets packaged, so a packaging leg no longer pays for a full Debug compile it then + # throws away. macOS and Windows have always tested their release build. + # + # Instrumented and debug-test legs reach this step only when they package (the sanitizer + # leg currently does, the coverage leg does not); their suite already ran in the debug + # step above, so the tests are not repeated here. + - name: "Build release, test and package" id: build - if: matrix.package != 'skip' + if: matrix.package != 'skip' || env.RUN_TESTS == 'true' shell: bash env: SUFFIX: '-${{ matrix.distro }}${{ matrix.version }}' package: '${{ matrix.package }}' server_only: '${{ matrix.server_only }}' - WITH_CRASH_REPORTING: 'ON' + # Only a leg that packages builds the shipping configuration; a release build that + # only runs tests stays free of the crash reporting SDK. + WITH_CRASH_REPORTING: ${{ matrix.package != 'skip' && 'ON' || '' }} # Tag builds only: artifacts from master pushes and PR branches must not report to # Sentry. Forks get no secrets on pull_request, so they keep the mock fallback either way. # configure.outputs.tag is set for ANY tag, so beta (Development) tags get the DSN @@ -287,6 +314,7 @@ jobs: source .ci/docker.sh args=() [[ $server_only == yes ]] && args+=(--no-client) + [[ $RUN_TESTS == true ]] && args+=(--test) [[ $GITHUB_REF == "refs/heads/master" ]] && args+=(--evict-ccache "$CCACHE_EVICTION_AGE") args+=(--ccache "$CCACHE_SIZE") args+=(--cmake-generator "$CMAKE_GENERATOR") @@ -296,7 +324,14 @@ jobs: # configures, and empty keeps the mock fallback. export RUN_ARGS="${RUN_ARGS:-} -e COCKATRICE_SENTRY_DSN" - RUN --server --release --package "$package" "${args[@]}" + # --package takes an optional type, so the flag and its argument have to travel + # together or the type would become the next flag. + package_args=() + [[ -n $package && $package != skip ]] && package_args+=(--package "$package") + + # compile.sh runs the tests before the package target, so a failing suite stops the + # leg before anything is built into an installer. + RUN --server --release "${package_args[@]}" "${args[@]}" - name: "Install sentry-cli" if: matrix.package != 'skip' && needs.configure.outputs.tag != null