diff --git a/.github/workflows/ci-bazel-testreports.yml b/.github/workflows/ci-bazel-testreports.yml index b715d852..02990454 100644 --- a/.github/workflows/ci-bazel-testreports.yml +++ b/.github/workflows/ci-bazel-testreports.yml @@ -95,18 +95,35 @@ jobs: LCOV_COBERTURA_SCRIPT=$(python3 -c "import importlib.util; print(importlib.util.find_spec('lcov_cobertura.lcov_cobertura').origin)") # TODO: change to output coverage.xml, not coverage.dat.xml, and update the rest of the workflow accordingly. - find ./ -name 'coverage.dat' -exec python3 "$LCOV_COBERTURA_SCRIPT" {} --output {}.xml \; + # Skip empty coverage.dat files (e.g. from a target that instruments no workspace sources), + # since lcov_cobertura would produce an empty XML that CodeCoverageSummary cannot parse. + find ./ -name 'coverage.dat' -not -empty -exec python3 "$LCOV_COBERTURA_SCRIPT" {} --output {}.xml \; - name: Upload Cobertura Coverage Report uses: actions/upload-artifact@v4 with: name: cobertura-coverage-results-${{ matrix.place }}-${{ matrix.short_name }} test${{ matrix.os }} path: './**/coverage.dat.xml' + # No XML is produced when all coverage.dat files were empty (e.g. a target that instruments + # no workspace sources), so tolerate the case where there are no files to upload. + if-no-files-found: warn if: always() && env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') # Run even if the previous step fails + - name: Check for Cobertura XML files + id: check_cobertura + if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') + run: | + # Empty coverage.dat files (e.g. a target that instruments no workspace sources) are skipped + # by the conversion step, so there may be no XML to summarise. + if find ./ -name 'coverage.dat.xml' | grep -q .; then + echo "found=true" >> "$GITHUB_OUTPUT" + else + echo "found=false" >> "$GITHUB_OUTPUT" + fi + - name: Code Coverage Report uses: irongut/CodeCoverageSummary@v1.3.0 - if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') # Run only if coverage is enabled and not on Windows artifacts. + if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') && steps.check_cobertura.outputs.found == 'true' with: filename: ./*/coverage.dat.xml badge: true @@ -120,12 +137,12 @@ jobs: thresholds: '60 80' - name: Post coverage report as comment uses: marocchino/sticky-pull-request-comment@v2 - if: github.event_name == 'pull_request' && env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') # Run only on PRs and if coverage is enabled and not on Windows artifacts. + if: github.event_name == 'pull_request' && env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') && steps.check_cobertura.outputs.found == 'true' with: recreate: true header: coverage path: code-coverage-results.md - name: Add coverage report to job summary - if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') # Run only if coverage is enabled and not on Windows artifacts. + if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') && steps.check_cobertura.outputs.found == 'true' run: | cat code-coverage-results.md >> "$GITHUB_STEP_SUMMARY" diff --git a/BUILD b/BUILD index dc480d6a..9cf41c44 100644 --- a/BUILD +++ b/BUILD @@ -268,13 +268,36 @@ cc_test( ) cc_test( - # This test intentionally links nothing except gtest. name = "minimal_test", size = "small", srcs = ["minimal_test.cpp"], deps = [ + ":util", "@com_google_googletest//:gtest_main", ], + linkopts = select({ + # TODO: It is unclear why the CI runner reports host_cpu=x64_windows + # rather than x64_windows_msvc even when using MSVC. This may be a + # Bazel version- or toolchain-configuration-specific behaviour. Until + # this is understood and validated, keep :windows and :windows_msvc + # cases in sync so that either selector covers the MSVC toolchain. + ":windows": [ + "/DEFAULTLIB:shell32.lib", # openurl() needs ShellExecute + "/SUBSYSTEM:CONSOLE", # ensure gtest main resolves to main(), not WinMain + # TODO: /SUBSYSTEM:CONSOLE is currently overridden by the transitive + # /SUBSYSTEM:WINDOWS linkopt that rules_libsdl12 injects for all + # non-debug Windows builds. Fix this by adding a console-subsystem + # variant target in rules_libsdl12 that unit tests can link against + # instead of the full SDL library (which forces SUBSYSTEM:WINDOWS). + ], + ":windows_msvc": [ + "/DEFAULTLIB:shell32.lib", # openurl() needs ShellExecute + "/SUBSYSTEM:CONSOLE", # ensure gtest main resolves to main(), not WinMain + # TODO: same as :windows case above -- see comment there. + ], + ":windows_msys": ["-lshell32"], + "//conditions:default": ["-ldl"], + }), ) cc_library( diff --git a/minimal_test.cpp b/minimal_test.cpp index 4e8952d5..4f65ffe9 100644 --- a/minimal_test.cpp +++ b/minimal_test.cpp @@ -20,15 +20,34 @@ #include #include +#include "util.h" + +#ifdef _WIN32 +// It appears that rules_libsdl12 may be setting /SUBSYSTEM:WINDOWS as a +// transitive linkopt on Windows non-debug builds, which would cause the CRT +// to require WinMain rather than main. This WinMain shim delegates to main() +// so that gtest_main works correctly even when SUBSYSTEM:WINDOWS is forced. +// TODO: The proper fix is for rules_libsdl12 to expose a console-subsystem +// variant target that unit tests can link against without inheriting the +// SUBSYSTEM:WINDOWS linkopt. Until then, this shim is required. +#define WIN32_LEAN_AND_MEAN +#include +#include // provides __argc/__argv with correct dllimport linkage +// Forward-declare main() which is provided by gtest_main. +int main(int argc, char** argv); +int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, + PSTR lpCmdLine, int nCmdShow) { + (void)hInstance; (void)hPrevInstance; (void)lpCmdLine; (void)nCmdShow; + return main(__argc, __argv); +} +#endif // Demonstrate some basic assertions. TEST(StrReplaceTest, BasicAssertions) { - auto haystack = std::string("hello world"); - auto needle = std::string("hello"); - auto replace = std::string("hi"); - - auto got = - haystack.replace(haystack.find(needle), needle.size(), replace); + auto got = str_replace( + std::string("hello"), + std::string("hi"), + std::string("hello world")); auto want = "hi world"; EXPECT_STREQ(got.c_str(), want);