fix: workaround Bazel lcov parsing bug (#521)

bazelbuild/bazel#25118 was causing our
coverage reports to fail to parse into combined report.

Related to #520 which was missing testing that the report actually
worked.
fyi @dizzy57

.github/workflows/ci.yaml

@@ -67,17 +67,18 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - run: ./minimal_download_test.sh
-      - run: bazel coverage //src/...
+      - run: bazel coverage --combined_report=lcov //src/...
       - uses: hrishikesh-kadam/setup-lcov@6c1aa0cc9e1c02f9f58f01ac599f1064ccc83470 # v1
       # The github-actions-report-lcov doesn't follow symlinks, so get an absolute path
-      - run: echo "bazel_testlogs=$(bazel info bazel-testlogs)" >> $GITHUB_ENV
+      - run: echo "output_path=$(bazel info output_path)" >> $GITHUB_ENV
       - name: Report code coverage
         if: github.event.pull_request.head.repo.fork == false # Forks always have read-only tokens
         uses: zgosalvez/github-actions-report-lcov@5989987f8058a03137e90bc16f9c0baaac5e069a # v4.1.22
         with:
           title-prefix: "e2e/use_release folder:"
-          working-directory: ${{ env.bazel_testlogs }}
-          coverage-files: "**/coverage.dat"
+          # Point to the already-merged data file Bazel produces with --combined_report=lcov
+          # Follows https://bazel.build/configure/coverage#running_coverage
+          coverage-files: "${{ env.output_path }}/_coverage/_coverage_report.dat"
           github-token: ${{ secrets.GITHUB_TOKEN }}
           update-comment: true
 

py/private/py_binary.bzl

@@ -175,13 +175,6 @@ A collision can occur when multiple packages providing the same file are install
     "_allowlist_function_transition": attr.label(
         default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
     ),
-    # Magic attribute to make coverage work. There's no
-    # docs about this; see TestActionBuilder.java
-    "_lcov_merger": attr.label(
-        default = configuration_field(fragment = "coverage", name = "output_generator"),
-        executable = True,
-        cfg = "exec",
-    ),
 })
 
 _attrs.update(**_py_library.attrs)
@@ -191,6 +184,16 @@ _test_attrs = dict({
         doc = "Specifies additional environment variables to inherit from the external environment when the test is executed by bazel test.",
         default = [],
     ),
+    # Magic attribute to make coverage --combined_report flag work.
+    # There's no docs about this.
+    # See https://github.com/bazelbuild/bazel/blob/fde4b67009d377a3543a3dc8481147307bd37d36/tools/test/collect_coverage.sh#L186-L194
+    # NB: rules_python ALSO includes this attribute on the py_binary rule, but we think that's a mistake.
+    # see https://github.com/aspect-build/rules_py/pull/520#pullrequestreview-2579076197
+    "_lcov_merger": attr.label(
+        default = configuration_field(fragment = "coverage", name = "output_generator"),
+        executable = True,
+        cfg = "exec",
+    ),
 })
 
 def _python_version_transition_impl(_, attr):

py/private/pytest.py.tmpl

@@ -93,7 +93,23 @@ if __name__ == "__main__":
     elif cov:
         cov.stop()
         # https://bazel.build/configure/coverage
-        cov.lcov_report(outfile = os.getenv("COVERAGE_OUTPUT_FILE"))
+        coverage_output_file = os.getenv("COVERAGE_OUTPUT_FILE")
+
+        # Workaround https://github.com/bazelbuild/bazel/issues/25118
+        # by removing 'end line number' from FN: records
+        unfixed_dat = coverage_output_file + ".tmp"
+        cov.lcov_report(outfile = unfixed_dat)
         cov.save()
+
+        with open(unfixed_dat, "r") as unfixed:
+          with open(coverage_output_file, "w") as output_file:
+            for line in unfixed:
+              if line.startswith('FN:'):
+                parts = line[3:].split(",")  # Remove 'FN:' and split by commas
+                if len(parts) == 3:
+                  output_file.write(f"FN:{parts[0]},{parts[2]}")
+                  continue
+              output_file.write(line)
+        os.unlink(unfixed_dat)
 
     sys.exit(exit_code)