Merge pull request #5817 from sysown/feature/ci-codecov-unit-tests

ci(coverage): upload unit-test LCOV to Codecov

Author: renecannao

.github/workflows/CI-unit-tests-asan-coverage.yml

@@ -42,7 +42,15 @@ on:
     types: [ completed ]
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.event.workflow_run && github.event.workflow_run.head_branch || github.ref_name }}
+  # Include `github.event_name` so a workflow_run-triggered run never
+  # falls into the same group as a workflow_dispatch run on the same
+  # branch. Without this, dispatching on a feature branch while a
+  # workflow_run for v3.0 is also active cancels the dispatch even
+  # though the branches differ -- GitHub's concurrency comparison
+  # appears to ignore the head_branch suffix in practice. Two
+  # workflow_runs for the same branch still serialize via this group;
+  # so do two dispatches for the same branch.
+  group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.workflow_run && github.event.workflow_run.head_branch || github.ref_name }}
   cancel-in-progress: true
 
 env:
@@ -61,6 +69,17 @@ jobs:
     if: ${{ github.event.workflow_run && github.event.workflow_run.conclusion == 'success' || ! github.event.workflow_run }}
     runs-on: ubuntu-24.04
     timeout-minutes: 120
+    # codecov/codecov-action@v4 needs `id-token: write` to mint a
+    # GitHub OIDC token for tokenless uploads against the Codecov
+    # GitHub App. Without this, the upload falls back to legacy
+    # token-based auth and fails on protected target branches with
+    # `HTTP 400: Token required because branch is protected` even
+    # though the app is installed. We keep `contents: read` (the
+    # GitHub default) explicit so granting id-token: write here
+    # doesn't implicitly widen any other scope.
+    permissions:
+      contents: read
+      id-token: write
 
     steps:
 
@@ -141,6 +160,37 @@ jobs:
         path: coverage/lcov.info
         if-no-files-found: warn
 
+    - name: Upload coverage to Codecov
+      # Send the same lcov.info we already archive as a workflow artifact
+      # to Codecov so PRs get the "this PR changes coverage of touched
+      # files from N% to M%" comment and main accumulates a historical
+      # graph at https://app.codecov.io/gh/sysown/proxysql.
+      #
+      # `if: always()` so coverage uploads regardless of whether the unit
+      # tests themselves passed; partial coverage is still useful for
+      # diagnosing why a PR went red. `fail_ci_if_error: false` so a
+      # transient Codecov outage never gates a green CI run on a
+      # third-party SaaS.
+      if: always()
+      uses: codecov/codecov-action@v4
+      with:
+        files: coverage/lcov.info
+        flags: unit-tests
+        name: unit-tests-asan-coverage
+        # Tokenless upload via GitHub OIDC. Codecov treats every branch
+        # as "protected" by default and rejects unauthenticated uploads
+        # with HTTP 400 "Token required because branch is protected" --
+        # this is Codecov's own branch-protection concept, unrelated to
+        # GitHub's. The fix is `use_oidc: true`, which makes the action
+        # mint a GitHub OIDC token (granted by `permissions:
+        # id-token: write` on this job) and present it to Codecov in
+        # place of a static upload token. Without `use_oidc: true` the
+        # action silently falls back to legacy tokenless mode and the
+        # upload fails.
+        use_oidc: true
+        fail_ci_if_error: false
+        verbose: true
+
     - name: Upload coverage HTML report
       if: always()
       uses: actions/upload-artifact@v4

lib/MySQL_Session.cpp

@@ -1863,9 +1863,15 @@ bool MySQL_Session::handler_again___verify_backend_multi_statement() {
  * status values.
  * @note This method is primarily used to maintain a history of the session's previous states for later reference or
  * recovery purposes.
- * @note The LCOV_EXCL_START and LCOV_EXCL_STOP directives are used to exclude the assert statement from code coverage
- * analysis because the condition should not occur during normal execution and is included as a safeguard against
- * programming errors.
+ * @note The lcov exclude-block directives wrapping the `assert(0)` below
+ * remove that line from coverage analysis -- the condition should never
+ * occur during normal execution and is only there as a safeguard against
+ * programming errors. (The literal directive strings are deliberately not
+ * spelled out in this comment because lcov's geninfo scans every source
+ * line for them and treats the doc occurrence as a real exclude marker,
+ * which double-opens the exclude block on the next assert and breaks
+ * coverage capture for the whole file with a "overlapping exclude
+ * directives" error.)
  */
 void MySQL_Session::set_previous_status_mode3(bool allow_execute) {
 	switch(status) {

lib/PgSQL_Session.cpp

@@ -6378,9 +6378,15 @@ bool PgSQL_Session::is_in_transaction() const {
  * status values.
  * @note This method is primarily used to maintain a history of the session's previous states for later reference or
  * recovery purposes.
- * @note The LCOV_EXCL_START and LCOV_EXCL_STOP directives are used to exclude the assert statement from code coverage
- * analysis because the condition should not occur during normal execution and is included as a safeguard against
- * programming errors.
+ * @note The lcov exclude-block directives wrapping the `assert(0)` below
+ * remove that line from coverage analysis -- the condition should never
+ * occur during normal execution and is only there as a safeguard against
+ * programming errors. (The literal directive strings are deliberately not
+ * spelled out in this comment because lcov's geninfo scans every source
+ * line for them and treats the doc occurrence as a real exclude marker,
+ * which double-opens the exclude block at line 6399 and breaks coverage
+ * capture for the whole file with a "overlapping exclude directives"
+ * error.)
  */
 void PgSQL_Session::set_previous_status_mode3(bool allow_execute) {
 	switch (status) {

test/infra/control/run-unit-tests-asan-coverage.bash

@@ -63,10 +63,22 @@ echo "==> Capturing baseline coverage snapshot (--initial)"
 # coverage for every instrumented source line, so unrun code paths
 # show as 0% in the merged report (rather than being absent
 # entirely).
+#
+# Capture from lib/ only. src/ contains the proxysql binary's
+# main.cpp and a small number of helpers; unit tests link only
+# against libproxysql.a (from lib/) so there is never a .gcda in
+# src/ from this workflow. Keeping `--directory src` here makes
+# lcov 2.x abort with "no .gcda files found in src" (an `empty`-
+# class error, not covered by --ignore-errors gcov,source), which
+# wipes the output file and breaks the whole capture chain.
+#
+# `empty` is added to --ignore-errors as belt-and-suspenders so a
+# future test reorganisation that produces a partially-empty
+# directory tree doesn't recur the same silent failure.
 lcov --quiet --capture --initial \
-     --directory lib --directory src \
+     --directory lib \
      --output-file coverage/lcov-base.info \
-     --ignore-errors gcov,source || true
+     --ignore-errors gcov,source,empty || true
 
 echo "==> Running unit tests under ASAN"
 # Iterate every executable under test/tap/tests/unit/. We
@@ -107,10 +119,13 @@ if [ ${#FAILED[@]} -gt 0 ]; then
 fi
 
 echo "==> Capturing post-test coverage"
+# Mirror the baseline-capture scope above: lib/ only, empty added to
+# --ignore-errors. See the baseline-capture comment for the full
+# rationale.
 lcov --quiet --capture \
-     --directory lib --directory src \
+     --directory lib \
      --output-file coverage/lcov-tests.info \
-     --ignore-errors gcov,source,mismatch || true
+     --ignore-errors gcov,source,mismatch,empty || true
 
 if [ -s coverage/lcov-base.info ] && [ -s coverage/lcov-tests.info ]; then
     lcov --quiet \