Fix minimal_test: wire up str_replace from util.h instead of stdlib

.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 <packages/> 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"

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(

minimal_test.cpp

@@ -20,20 +20,39 @@
 
 #include <string>
 #include <gtest/gtest.h>
+#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 <windows.h>
+#include <stdlib.h>  // 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);
 }
 
 TEST(EqualityTest, BasicAssertions) {
   // Expect equality.
   EXPECT_EQ(7 * 6, 42);