From a264e9a76336816c5dae908a5da4c32e42240cbe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 01:04:37 +0000 Subject: [PATCH 1/4] Initial plan From dc71082de66eff6f27c2936a89cede996c9527a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 01:15:26 +0000 Subject: [PATCH 2/4] Fix: skip empty LCOV files and guard coverage summary steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `-not -empty` to `find` in the lcov→Cobertura conversion step so that zero-byte coverage.dat files (produced by targets like minimal_test that have no project source files) are not converted; lcov_cobertura would otherwise emit `` (empty), causing irongut/CodeCoverageSummary to fail with "No package data found". - Add a "Check for Cobertura XML files" step (id: check_cobertura) that sets `found=true/false` output so downstream steps can skip cleanly instead of failing when no XML was produced. - Gate "Code Coverage Report", "Post coverage report as comment", and "Add coverage report to job summary" on `steps.check_cobertura.outputs.found == 'true'` so they are skipped (not failed) when there is nothing to summarise. Co-authored-by: ivucica <477596+ivucica@users.noreply.github.com> --- .github/workflows/ci-bazel-testreports.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-bazel-testreports.yml b/.github/workflows/ci-bazel-testreports.yml index b715d852..a6aa63e1 100644 --- a/.github/workflows/ci-bazel-testreports.yml +++ b/.github/workflows/ci-bazel-testreports.yml @@ -95,7 +95,9 @@ 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 targets with no project source files like minimal_test), + # 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 @@ -104,9 +106,21 @@ jobs: path: './**/coverage.dat.xml' 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. from minimal_test which has no project 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 +134,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" From fe1a84a7457f086683735d27530374383c4b5829 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 15:30:31 +0000 Subject: [PATCH 3/4] ci: add if-no-files-found: warn to cobertura upload step When all coverage.dat files are empty (e.g. minimal_test has no project sources and produces a 0-byte coverage.dat), the -not -empty filter means no coverage.dat.xml files are produced. The upload step would otherwise implicitly rely on the default if-no-files-found behaviour; make the intent explicit with if-no-files-found: warn. Co-authored-by: ivucica <477596+ivucica@users.noreply.github.com> --- .github/workflows/ci-bazel-testreports.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci-bazel-testreports.yml b/.github/workflows/ci-bazel-testreports.yml index a6aa63e1..92811bf0 100644 --- a/.github/workflows/ci-bazel-testreports.yml +++ b/.github/workflows/ci-bazel-testreports.yml @@ -104,6 +104,9 @@ jobs: 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. minimal_test has no project + # 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 From 2a6d33fc346115fd246880162d36b8343b5ea24b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:29:08 +0000 Subject: [PATCH 4/4] Fix: minimal_test should call project str_replace, not stdlib minimal_test.cpp was using stdlib haystack.replace() instead of the project's str_replace() from util.h. As a result it never needed :util as a dep; the BUILD comment "This test intentionally links nothing except gtest" described this incorrect state. Consequences: - The test never exercised any project code - bazel coverage produced an empty coverage.dat (no workspace sources were instrumented), causing irongut/CodeCoverageSummary to fail with "No package data found" Changes: - minimal_test.cpp: add #include "util.h", WinMain shim (same as util_test.cpp), and update StrReplaceTest to call str_replace() - BUILD: add :util dep and same linkopts as util_test; remove the misleading "intentionally links nothing" comment - ci-bazel-testreports.yml: update comments that cited minimal_test as an example of a target with no project sources (now hypothetical) Co-authored-by: ivucica <477596+ivucica@users.noreply.github.com> --- .github/workflows/ci-bazel-testreports.yml | 8 +++--- BUILD | 25 ++++++++++++++++- minimal_test.cpp | 31 +++++++++++++++++----- 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci-bazel-testreports.yml b/.github/workflows/ci-bazel-testreports.yml index 92811bf0..02990454 100644 --- a/.github/workflows/ci-bazel-testreports.yml +++ b/.github/workflows/ci-bazel-testreports.yml @@ -95,7 +95,7 @@ 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. - # Skip empty coverage.dat files (e.g. from targets with no project source files like minimal_test), + # 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 \; @@ -104,8 +104,8 @@ jobs: 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. minimal_test has no project - # sources), so tolerate the case where there are no files to upload. + # 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 @@ -113,7 +113,7 @@ jobs: id: check_cobertura if: env.ENABLE_BAZEL_COVERAGE == 'true' && !contains(matrix.os, 'windows') run: | - # Empty coverage.dat files (e.g. from minimal_test which has no project sources) are skipped + # 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" 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);